Search code examples
node.jsoracle-databasenode-oracledb

ORA-01036: illegal variable name/number when running query through nodejs


I am running this simple query 'ALTER USER hr IDENTIFIED BY secret' using oracledb nodejs package. Not sure why I am getting illegal variable name error. Can I do something like this "ALTER USER :user IDENTIFIED BY :password";if yes then what will be correct syntax for me

function get(req, res, next) {
    oracledb.getConnection(
        config.database,
        function(err, connection){
            if (err) {
                return next(err);
            }

            connection.execute(
                'ALTER USER :user IDENTIFIED BY :password'+

            {
                user: req.body.user
            },
            {
                password: req.body.password
            },

                {
                    outFormat: oracledb.OBJECT
                }


          });
        }

Thanks for the help


Solution

  • As Matthew pointed out, you'll have to use string concatenation and protect against SQL injection.

    To some degree, I question the need to do this. I know you're already aware of this post: https://jsao.io/2015/06/authentication-with-node-js-jwts-and-oracle-database/

    Because that solution uses a table to store user credentials, rather than depending on database users, the values can be safely bound in without worrying about SQL injection. Why not use that approach instead?

    Using database users with dynamic SQL requires that single quotes (') not be allowed in passwords. Frankly, I hate those kinds of restrictions. Password rules should exist to increase security (what must be included), not ensure that code can be executed safely (what's not allowed). That would not be an issue with a custom table.

    However, just so you have one, here's an example solution that's based on promises that shows how to use dbms_assert to sanitize the values coming in:

    const oracledb = require('oracledb');
    const config = require('./dbConfig.js');
    
    function get(req, res, next) {
      let conn;
      const user = req.body.user;
      const password = req.body.password; // Do not change case
    
      oracledb.getConnection(config)
        .then((c) => {
          conn = c;
    
          return conn.execute(
           `declare
    
              -- Use dbms_assert to sanitize values coming in to avoid SQL injection.
              l_user      varchar2(30) := dbms_assert.simple_sql_name(:user);
              l_password  varchar2(30) := dbms_assert.enquote_literal(:password);
              l_statement varchar2(100);
    
            begin
    
              -- Replace single quotes added by enquote_literal to left and right sides with double quotes
              l_password := '"' || substr(substr(l_password, 2, length(l_password)), 1, length(l_password) - 2) || '"';
    
              l_statement := 'alter user ' || l_user || ' identified by ' || l_password;
    
              execute immediate l_statement;
    
            end;`,
            {
              user: user,
              password: password
            }
          );
        })
        .then(result => {
          console.log('Password changed');
    
          // write to res
        })
        .catch(err => {
          console.log('Error changing password', err);
    
          next(err);
        })
        .then(() => {
          if (conn) { // conn assignment worked, need to close
            return conn.close();
          }
        })
        .catch(err => {
          console.log('Error during close', err);
        });
    }
    
    // Simulate run of 'get' function
    get(
      {
        body: {
          user: 'movie_budget',
          password: 'N0rm@l-P@sswOrd!' // This value will throw an error: '\'\; drop table users;'
        }
      }, 
      {},
      function() {}
    );
    

    Finally, combining database logic with controller logic may lead to code that's difficult to maintain. Have a look at this recording for some tips on organizing things a little better: https://www.youtube.com/watch?v=hQgw2WmyuFM