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

Question

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

Answer 1

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

Answer 2

There are several problems here:

1. There are missing spaces in the SQL string so various parts of it run together.

2. In any case the syntax is just ALTER USER hr IDENTIFIED BY secret, as you've said in the introduction; I don't think the WHERE clause will have any effect.

3. You can't use bind variables for things like user names in ALTER statements. Instead you have to construct the command by concatenating strings, and check carefully that the components are well-formed and can't be used for a SQL injection attack.

Something like this should work (but I haven't tested it):

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

            user = req.body.user.toLowerCase();
            password = req.body.password.toLowerCase();

            // TODO make sure user and password are safe from SQL injections here
            // in particular, that they don't contain single quotation characters (')
            // or spaces

            connection.execute(
                'ALTER USER ' + user + ' IDENTIFIED BY ' + password
            );
        }
    )
}