Merge pull request #32 from hapipal/sqli-fix

Fix for SQL injection vulnerability related to handling of sort query
hapipal · Feb 24, 2023 · 2245b50 · 2245b50
2 parents 712756e + b457c2d
commit 2245b50
Show file tree

Hide file tree

Showing 5 changed files with 343 additions and 11 deletions.
diff --git a/lib/action-util.js b/lib/action-util.js
@@ -1,5 +1,6 @@
 'use strict';
 
+const Boom = require('@hapi/boom');
 const Hoek = require('@hapi/hoek');
 
 const internals = {};
@@ -84,23 +85,59 @@ module.exports = (request, options) => {
          * @param  {Request} request
          * @param  {Object} options
          */
-        parseSort: () => {
+        parseSort: (model) => {
 
-            return request.query.sort || options.sort || undefined;
+            if (request.query.sort) {
+                const sortQueryParam = request.query.sort;
+                const orderedSort = sortQueryParam.split(' ');
+                const validKeys = model.joiSchema.describe().keys;
+
+                if (orderedSort.length === 1) {
+
+                    if (validKeys[sortQueryParam]) {
+                        return sortQueryParam;
+                    }
+                }
+                else if (orderedSort.length === 2) {//if param is of style "id" ASC
+                    const key = orderedSort[0].replace(/(^"|"$)/g, '');//strip leading/trailing quotes
+                    if (['asc','desc'].includes(orderedSort[1].toLowerCase()) && validKeys[key]) {
+                        return sortQueryParam;
+                    }
+                }
+
+                throw Boom.badRequest('Sort query param must be a present in the model\'s Joi schema and sort order must be asc or desc');
+
+            }
+
+            return options.sort || undefined;
         },
 
         /**
          * @param  {Request} request
          * @param  {Object} options
          */
-        parseWhere: () => {
+        parseWhere: (model) => {
 
             let queryWhere;
-            if ( request.query.where ) {
-                queryWhere = JSON.parse(request.query.where );
+            if (request.query.where) {
+                const validKeys = model.joiSchema.describe().keys;
+                try {
+                    queryWhere = JSON.parse(request.query.where);
+                }
+                catch (ignoreError) {
+                    throw Boom.badRequest('Unable to parse where query parameter');
+                }
+
+                const queryKeys = Object.keys(queryWhere);
+
+                if (queryKeys.every((k) => validKeys[k])) {
+                    return queryWhere;
+                }
+
+                throw Boom.badRequest('Where query param must be a present in the model\'s Joi schema');
             }
 
-            return queryWhere || options.where || undefined;
+            return options.where || undefined;
         },
 
         /**

diff --git a/lib/actions/find.js b/lib/actions/find.js
@@ -34,10 +34,10 @@ module.exports = function findRecords(route, options) {
         }
 
         const limit = actionUtil.parseLimit();
-        const sort = actionUtil.parseSort();
+        const sort = actionUtil.parseSort(Model);
         const rangeStart = actionUtil.parseSkip();
         const rangeEnd = rangeStart ? rangeStart + limit : limit;
-        const where = actionUtil.parseWhere();
+        const where = actionUtil.parseWhere(Model);
 
         const foundModelsQuery = Model.query().range(rangeStart, rangeEnd).limit(limit);
 

diff --git a/lib/actions/populate.js b/lib/actions/populate.js
@@ -33,7 +33,7 @@ module.exports = function expand(route, options) {
 
         const keys = actionUtil.getKeys();
         const limit = actionUtil.parseLimit();
-        const sort = actionUtil.parseSort();
+        const sort = actionUtil.parseSort(Model);
         const rangeStart = actionUtil.parseSkip();
         const rangeEnd = rangeStart + limit;
 

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@hapipal/tandy",
-  "version": "3.1.0",
+  "version": "3.1.1",
   "description": "Auto-generated, RESTful, CRUDdy route handlers for Schwifty models in hapi",
   "main": "lib/index.js",
   "scripts": {