fix(find): check for additional cases with order

changelog.md

@@ -4,6 +4,7 @@
 - [BUG] No longer crahes on `where: []`
 - [BUG] No longer crahes on `where: []`
 - [FEATURE] Validations are now enabled by default for upsert.
 - [FEATURE] Validations are now enabled by default for upsert.
 - [FEATURE] Preliminary support for `include.through.where`
 - [FEATURE] Preliminary support for `include.through.where`
+- [SECURITY/BUG] Fixed injection issue in direction param for order
 
 
 # 2.0.0-rc7
 # 2.0.0-rc7
 - [FEATURE] Throw an error if no where clause is given to `Model.destroy()`.
 - [FEATURE] Throw an error if no where clause is given to `Model.destroy()`.

lib/dialects/abstract/index.js

@@ -10,6 +10,7 @@ AbstractDialect.prototype.supports = {
   'VALUES ()': false,
   'VALUES ()': false,
   'LIMIT ON UPDATE': false,
   'LIMIT ON UPDATE': false,
   'ON DUPLICATE KEY': true,
   'ON DUPLICATE KEY': true,
+  'ORDER NULLS': false,
 
 
   /* What is the dialect's keyword for INSERT IGNORE */
   /* What is the dialect's keyword for INSERT IGNORE */
   'IGNORE': '',
   'IGNORE': '',

lib/dialects/abstract/query-generator.js

@@ -667,7 +667,11 @@ module.exports = (function() {
         // add 1st string as quoted, 2nd as unquoted raw
         // add 1st string as quoted, 2nd as unquoted raw
         var sql = (i > 0 ? this.quoteIdentifier(tableNames.join('.')) + '.' : (Utils._.isString(obj[0]) ? this.quoteIdentifier(parent.name) + '.' : '')) + this.quote(obj[i], parent, force);
         var sql = (i > 0 ? this.quoteIdentifier(tableNames.join('.')) + '.' : (Utils._.isString(obj[0]) ? this.quoteIdentifier(parent.name) + '.' : '')) + this.quote(obj[i], parent, force);
         if (i < len - 1) {
         if (i < len - 1) {
-          sql += ' ' + obj[i + 1];
+          if (obj[i + 1]._isSequelizeMethod) {
+            sql += this.handleSequelizeMethod(obj[i + 1]);
+          } else {
+            sql += ' ' + obj[i + 1];
+          }
         }
         }
         return sql;
         return sql;
       } else if (obj._modelAttribute) {
       } else if (obj._modelAttribute) {
@@ -1264,7 +1268,18 @@ module.exports = (function() {
         var subQueryOrder = [];
         var subQueryOrder = [];
 
 
         var validateOrder = function(order) {
         var validateOrder = function(order) {
-          if (!_.contains(['ASC', 'DESC'], order.toUpperCase())) {
+          if (order instanceof Utils.literal) return;
+
+          if (!_.contains([
+            'ASC',
+            'DESC',
+            'ASC NULLS LAST',
+            'DESC NULLS LAST',
+            'ASC NULLS FIRST',
+            'DESC NULLS FIRST',
+            'NULLS FIRST',
+            'NULLS LAST'
+          ], order.toUpperCase())) {
             throw new Error(util.format('Order must be \'ASC\' or \'DESC\', \'%s\' given', order));
             throw new Error(util.format('Order must be \'ASC\' or \'DESC\', \'%s\' given', order));
           }
           }
         };
         };
@@ -1284,6 +1299,7 @@ module.exports = (function() {
             if (subQuery && (Array.isArray(t) && !(t[0] instanceof Model) && !(t[0].model instanceof Model))) {
             if (subQuery && (Array.isArray(t) && !(t[0] instanceof Model) && !(t[0].model instanceof Model))) {
               subQueryOrder.push(this.quote(t, model));
               subQueryOrder.push(this.quote(t, model));
             }
             }
+
             mainQueryOrder.push(this.quote(t, model));
             mainQueryOrder.push(this.quote(t, model));
           }.bind(this));
           }.bind(this));
         } else {
         } else {

lib/dialects/mssql/index.js

@@ -15,6 +15,7 @@ MssqlDialect.prototype.supports = _.merge(_.cloneDeep(Abstract.prototype.support
   'DEFAULT': true,
   'DEFAULT': true,
   'DEFAULT VALUES': true,
   'DEFAULT VALUES': true,
   'LIMIT ON UPDATE': true,
   'LIMIT ON UPDATE': true,
+  'ORDER NULLS': false,
   lock: false,
   lock: false,
   transactions: false,
   transactions: false,
   migrations: false,
   migrations: false,

lib/dialects/postgres/index.js

@@ -15,6 +15,7 @@ PostgresDialect.prototype.supports = _.merge(_.cloneDeep(Abstract.prototype.supp
   'DEFAULT VALUES': true,
   'DEFAULT VALUES': true,
   'EXCEPTION': true,
   'EXCEPTION': true,
   'ON DUPLICATE KEY': false,
   'ON DUPLICATE KEY': false,
+  'ORDER NULLS': true,
   returnValues: {
   returnValues: {
     returning: true
     returning: true
   },
   },

test/model/findAll/order.test.js

@@ -99,6 +99,26 @@ describe(Support.getTestDialectTeaser('Model'), function() {
           })).to.eventually.be.rejectedWith(Error, 'Order must be \'ASC\' or \'DESC\', \';DELETE YOLO INJECTIONS\' given');
           })).to.eventually.be.rejectedWith(Error, 'Order must be \'ASC\' or \'DESC\', \';DELETE YOLO INJECTIONS\' given');
         });
         });
 
 
+        if (current.dialect.supports['ORDER NULLS']) {
+          it('should not throw with on NULLS LAST/NULLS FIRST', function () {
+            return this.User.findAll({
+              include: [this.Group],
+              order: [
+                ['id', 'ASC NULLS LAST'],
+                [this.Group, 'id', 'DESC NULLS FIRST']
+              ]
+            });
+          });
+        }
+
+        it('should not throw on a literal', function () {
+          return this.User.findAll({
+            order: [
+              ['id', this.sequelize.literal('ASC, id DESC')]
+            ]
+          });
+        });
+
         it('should not throw with include when last order argument is a field', function () {
         it('should not throw with include when last order argument is a field', function () {
           return this.User.findAll({
           return this.User.findAll({
             include: [this.Group],
             include: [this.Group],