Security advisory GHSA-cgpp-wm2h-6hqx: SQL Injection in waterline-sequel
Package: waterline-sequel (npm), vulnerable version: 0.5.0, fixed in: 0.5.1
CWE: CWE-89
Files changed by the fix:
sequel/lib/criteriaProcessor.js
sequel/lib/utils.js

Fix diff:
diff --git a/sequel/lib/criteriaProcessor.js b/sequel/lib/criteriaProcessor.js
index c385450..d23ba04 100644
--- a/sequel/lib/criteriaProcessor.js
+++ b/sequel/lib/criteriaProcessor.js
@@ -726,7 +726,8 @@ CriteriaProcessor.prototype.prepareCriterion = function prepareCriterion(key, va
         str = comparator + ' ' + '$' + this.paramCount;
       }
       else {
-        str = comparator + ' ' + utils.escapeName(value, '"');
+        // Note that wildcards are not escaped out of like criterion intentionally
+        str = comparator + ' "' + utils.escapeString(value) + '"';
       }
 
       break;
@@ -750,7 +751,7 @@ CriteriaProcessor.prototype.prepareCriterion = function prepareCriterion(key, va
         str = comparator + ' ' + '$' + this.paramCount;
       }
       else {
-        str = comparator + ' ' + utils.escapeName('%' + value + '%', '"');
+        str = comparator + ' "%' + utils.escapeString(value, true) + '%"';
       }
 
       break;
@@ -774,7 +775,7 @@ CriteriaProcessor.prototype.prepareCriterion = function prepareCriterion(key, va
         str = comparator + ' ' + '$' + this.paramCount;
       }
       else {
-        str = comparator + ' ' + utils.escapeName(value + '%', '"');
+        str = comparator + ' "' + utils.escapeString(value, true) + '%"';
       }
 
       break;
@@ -798,7 +799,7 @@ CriteriaProcessor.prototype.prepareCriterion = function prepareCriterion(key, va
         str = comparator + ' ' + '$' + this.paramCount;
       }
       else {
-        str = comparator + ' ' + utils.escapeName('%' + value, '"');
+        str = comparator + ' "%' + utils.escapeString(value, true) + '"';
       }
 
       break;
@@ -841,7 +842,7 @@ CriteriaProcessor.prototype.skip = function(options) {
 CriteriaProcessor.prototype.sort = function(options) {
   var keys = Object.keys(options);
   if (!keys.length) { return; }
-  
+
   var self = this;
   this.queryString += ' ORDER BY ';
 
diff --git a/sequel/lib/utils.js b/sequel/lib/utils.js
index ce8911b..e858121 100644
--- a/sequel/lib/utils.js
+++ b/sequel/lib/utils.js
@@ -34,6 +34,12 @@ utils.object.hasOwnProperty = function(obj, prop) {
  *
  * Wraps a name in quotes to allow reserved
  * words as table or column names such as user.
+ *
+ *
+ * NOTE: do not use this method to escape strings in a general-purpose way.
+ * This is intended only to escape schema object (e.g. table and column) names.
+ * Check out utils.escapeString() for other purposes.  No harm in taking a
+ * peek at https://dev.mysql.com/doc/refman/5.7/en/identifiers.html .
  */
 
 utils.escapeName = function escapeName(name, escapeCharacter) {
@@ -139,10 +145,10 @@ utils.prepareValue = function(value) {
  * Escape Strings
  */
 
-utils.escapeString = function(value) {
+utils.escapeString = function(value, forLike) {
   if(!_.isString(value)) return value;
 
-  value = value.replace(/[\0\n\r\b\t\\\'\"\x1a]/g, function(s) {
+  value = value.replace(/[_%\0\n\r\b\t\\\'\"\x1a]/g, function(s) {
     switch(s) {
       case "\0": return "\\0";
       case "\n": return "\\n";
@@ -150,6 +156,8 @@ utils.escapeString = function(value) {
       case "\b": return "\\b";
       case "\t": return "\\t";
       case "\x1a": return "\\Z";
+      case "%": return forLike ? "\\%" : "%";
+      case "_": return forLike ? "\\_" : "_";
       default: return "\\"+s;
     }
   });
diff --git a/test/queries/complexSelectFilters.js b/test/queries/complexSelectFilters.js
index 99ebeb7..dc2c0e9 100644
--- a/test/queries/complexSelectFilters.js
+++ b/test/queries/complexSelectFilters.js
@@ -63,10 +63,10 @@ module.exports = {
     // Sequel.find()
     find  : {
 
-      // The queryString we expect to be rendered after calling `Sequel.select()`
+      // The queryString we expect to be rendered after calling `Sequel.find()`
       queryString    : 'SELECT `foo`.`color`, `foo`.`id`, `foo`.`createdAt`, `foo`.`updatedAt`, `foo`.`bar`, `foo`.`bat`, `foo`.`baz`, `__bat`.`color_g` AS `bat___color_g`, `__bat`.`color_h` AS `bat___color_h`, `__bat`.`color_i` AS `bat___color_i`, `__bat`.`id` AS `bat___id`, `__bat`.`createdAt` AS `bat___createdAt`, `__bat`.`updatedAt` AS `bat___updatedAt` FROM `foo` AS `foo`  LEFT OUTER JOIN `bat` AS `__bat` ON `foo`.`bat` = `__bat`.`id` WHERE `foo`.`bat` = 1 AND `foo`.`baz` IN (1,2,3,4) AND ((LOWER(`foo`.`color`) = "red") OR (LOWER(`foo`.`color`) = "blue") OR (LOWER(`foo`.`color`) = "grey") OR (LOWER(`foo`.`color`) > "111" )) ',
 
-      // The number of queries that will be returned after calling Sequel.select()
+      // The number of queries that will be returned after calling Sequel.find()
       queriesReturned: 1
     }
   }
diff --git a/test/queries/complexSelectNestedFilters.js b/test/queries/complexSelectNestedFilters.js
index 4538e53..1b762f6 100644
--- a/test/queries/complexSelectNestedFilters.js
+++ b/test/queries/complexSelectNestedFilters.js
@@ -86,10 +86,10 @@ module.exports = {
     // Sequel.find()
     find  : {
 
-      // The queryString we expect to be rendered after calling `Sequel.select()`
+      // The queryString we expect to be rendered after calling `Sequel.find()`
       queryString    : 'SELECT `foo`.`color`, `foo`.`id`, `foo`.`createdAt`, `foo`.`updatedAt`, `foo`.`bar`, `foo`.`bat`, `foo`.`baz`, `__bat`.`color_g` AS `bat___color_g`, `__bat`.`color_h` AS `bat___color_h`, `__bat`.`color_i` AS `bat___color_i`, `__bat`.`id` AS `bat___id`, `__bat`.`createdAt` AS `bat___createdAt`, `__bat`.`updatedAt` AS `bat___updatedAt` FROM `foo` AS `foo`  LEFT OUTER JOIN `bat` AS `__bat` ON `foo`.`bat` = `__bat`.`id` WHERE ((LOWER(`foo`.`color`) = "red") OR (LOWER(`foo`.`color`) = "blue") OR (LOWER(`foo`.`color`) = "grey") OR (`__bat`.`color_g` = "yellow" ) OR (`__bat`.`color_g` = "blue" )) AND `__bat`.`color_h` = "red" AND ((`__bat`.`color_i` IN ("pink","purple","green")) OR (`__bat`.`color_i` > "black" ) OR (`__bat`.`color_i` = "yellow"))  ',
 
-      // The number of queries that will be returned after calling Sequel.select()
+      // The number of queries that will be returned after calling Sequel.find()
       queriesReturned: 1
     }
   }
diff --git a/test/queries/escaped.js b/test/queries/escaped.js
new file mode 100644
index 0000000..9a72a4d
--- /dev/null
+++ b/test/queries/escaped.js
@@ -0,0 +1,148 @@
+module.exports = [
+  {
+
+    description: 'Should escape startsWith queries.',
+
+    table: 'foo',
+
+    query: {
+      where: {
+        color: { startsWith: '\\\\\\" OR 1=1; -- %_' }
+      }
+    },
+
+    // Expected results per query method.
+    expected: {
+
+      // Sequel.select()
+      select: {
+
+        // The queryString we expect to be rendered after calling `Sequel.select()`
+        queryString: 'SELECT `foo`.`color`, `foo`.`id`, `foo`.`createdAt`, `foo`.`updatedAt`, `foo`.`bar`, `foo`.`bat`, `foo`.`baz` FROM `foo` AS `foo` ',
+
+        // The number of queries that will be returned after calling Sequel.select()
+        queriesReturned: 1,
+      },
+
+      // Sequel.find()
+      find: {
+
+        // The queryString we expect to be rendered after calling Sequel.find()
+        queryString: 'SELECT `foo`.`color`, `foo`.`id`, `foo`.`createdAt`, `foo`.`updatedAt`, `foo`.`bar`, `foo`.`bat`, `foo`.`baz` FROM `foo` AS `foo`  WHERE LOWER(`foo`.`color`) LIKE "\\\\\\\\\\\\\\" or 1=1; -- \\%\\_%"  ',
+
+        // The number of queries that will be returned after calling Sequel.find()
+        queriesReturned: 1
+      }
+    }
+  },
+  {
+
+    description: 'Should escape endsWith queries.',
+
+    table: 'foo',
+
+    query: {
+      where: {
+        color: { endsWith: '\\\\\\" OR 1=1; -- %_' }
+      }
+    },
+
+    // Expected results per query method.
+    expected: {
+
+      // Sequel.select()
+      select: {
+
+        // The queryString we expect to be rendered after calling `Sequel.select()`
+        queryString: 'SELECT `foo`.`color`, `foo`.`id`, `foo`.`createdAt`, `foo`.`updatedAt`, `foo`.`bar`, `foo`.`bat`, `foo`.`baz` FROM `foo` AS `foo` ',
+
+        // The number of queries that will be returned after calling Sequel.select()
+        queriesReturned: 1,
+      },
+
+      // Sequel.find()
+      find: {
+
+        // The queryString we expect to be rendered after calling Sequel.find()
+        queryString: 'SELECT `foo`.`color`, `foo`.`id`, `foo`.`createdAt`, `foo`.`updatedAt`, `foo`.`bar`, `foo`.`bat`, `foo`.`baz` FROM `foo` AS `foo`  WHERE LOWER(`foo`.`color`) LIKE "%\\\\\\\\\\\\\\" or 1=1; -- \\%\\_"  ',
+
+        // The number of queries that will be returned after calling Sequel.find()
+        queriesReturned: 1
+      }
+    }
+  },
+  {
+
+    description: 'Should escape contains queries.',
+
+    table: 'foo',
+
+    query: {
+      where: {
+        color: { contains: '\\\\\\" OR 1=1; -- %_' }
+      }
+    },
+
+    // Expected results per query method.
+    expected: {
+
+      // Sequel.select()
+      select: {
+
+        // The queryString we expect to be rendered after calling `Sequel.select()`
+        queryString: 'SELECT `foo`.`color`, `foo`.`id`, `foo`.`createdAt`, `foo`.`updatedAt`, `foo`.`bar`, `foo`.`bat`, `foo`.`baz` FROM `foo` AS `foo` ',
+
+        // The number of queries that will be returned after calling Sequel.select()
+        queriesReturned: 1,
+      },
+
+      // Sequel.find()
+      find: {
+
+        // The queryString we expect to be rendered after calling Sequel.find()
+        queryString: 'SELECT `foo`.`color`, `foo`.`id`, `foo`.`createdAt`, `foo`.`updatedAt`, `foo`.`bar`, `foo`.`bat`, `foo`.`baz` FROM `foo` AS `foo`  WHERE LOWER(`foo`.`color`) LIKE "%\\\\\\\\\\\\\\" or 1=1; -- \\%\\_%"  ',
+
+        // The number of queries that will be returned after calling Sequel.find()
+        queriesReturned: 1
+      }
+    }
+  },
+  {
+
+    // Note that like queries are more permissive,
+    // not escaping wildcard (& and _) characters
+    description: 'Should escape like queries.',
+
+    table: 'foo',
+
+    query: {
+      where: {
+        color: { like: '\\\\\\" OR 1=1; -- %_' }
+      }
+    },
+
+    // Expected results per query method.
+    expected: {
+
+      // Sequel.select()
+      select: {
+
+        // The queryString we expect to be rendered after calling `Sequel.select()`
+        queryString: 'SELECT `foo`.`color`, `foo`.`id`, `foo`.`createdAt`, `foo`.`updatedAt`, `foo`.`bar`, `foo`.`bat`, `foo`.`baz` FROM `foo` AS `foo` ',
+
+        // The number of queries that will be returned after calling Sequel.select()
+        queriesReturned: 1,
+      },
+
+      // Sequel.find()
+      find: {
+
+        // The queryString we expect to be rendered after calling Sequel.find()
+        queryString: 'SELECT `foo`.`color`, `foo`.`id`, `foo`.`createdAt`, `foo`.`updatedAt`, `foo`.`bar`, `foo`.`bat`, `foo`.`baz` FROM `foo` AS `foo`  WHERE LOWER(`foo`.`color`) LIKE "\\\\\\\\\\\\\\" or 1=1; -- %_"  ',
+
+        // The number of queries that will be returned after calling Sequel.find()
+        queriesReturned: 1
+      }
+    }
+  }
+];
diff --git a/test/queries/index.js b/test/queries/index.js
index 00a5cfb..7a0a951 100644
--- a/test/queries/index.js
+++ b/test/queries/index.js
@@ -13,7 +13,7 @@ require("fs").readdirSync(__dirname + '/').forEach(function (file) {
     return;
   }
 
-  queries.push(query);
+  queries = queries.concat(query);
 });
 
 module.exports = queries;
diff --git a/test/queries/simpleSelect.js b/test/queries/simpleSelect.js
index cc820a0..cb1e09d 100644
--- a/test/queries/simpleSelect.js
+++ b/test/queries/simpleSelect.js
@@ -25,10 +25,10 @@ module.exports = {
     // Sequel.find()
     find  : {
 
-      // The queryString we expect to be rendered after calling `Sequel.select()`
+      // The queryString we expect to be rendered after calling `Sequel.find()`
       queryString    : 'SELECT `foo`.`color`, `foo`.`id`, `foo`.`createdAt`, `foo`.`updatedAt`, `foo`.`bar`, `foo`.`bat`, `foo`.`baz` FROM `foo` AS `foo`  ',
 
-      // The number of queries that will be returned after calling Sequel.select()
+      // The number of queries that will be returned after calling Sequel.find()
       queriesReturned: 1
     }
   }
diff --git a/test/queries/simpleSelectEmptySort.js b/test/queries/simpleSelectEmptySort.js
index 6323dcb..5d58844 100644
--- a/test/queries/simpleSelectEmptySort.js
+++ b/test/queries/simpleSelectEmptySort.js
@@ -28,10 +28,10 @@ module.exports = {
     // Sequel.find()
     find  : {
 
-      // The queryString we expect to be rendered after calling `Sequel.select()`
+      // The queryString we expect to be rendered after calling `Sequel.find()`
       queryString    : 'SELECT `foo`.`color`, `foo`.`id`, `foo`.`createdAt`, `foo`.`updatedAt`, `foo`.`bar`, `foo`.`bat`, `foo`.`baz` FROM `foo` AS `foo`  ',
 
-      // The number of queries that will be returned after calling Sequel.select()
+      // The number of queries that will be returned after calling Sequel.find()
       queriesReturned: 1
     }
   }
diff --git a/test/queries/simpleSelectNestedFilterAlias.js b/test/queries/simpleSelectNestedFilterAlias.js
index 24b92b8..fe0adf0 100644
--- a/test/queries/simpleSelectNestedFilterAlias.js
+++ b/test/queries/simpleSelectNestedFilterAlias.js
@@ -35,10 +35,10 @@ module.exports = {
     // Sequel.find()
     find  : {
 
-      // The queryString we expect to be rendered after calling `Sequel.select()`
+      // The queryString we expect to be rendered after calling `Sequel.find()`
       queryString    : 'SELECT `oddity`.`meta`, `oddity`.`id`, `oddity`.`createdAt`, `oddity`.`updatedAt`, `oddity`.`stubborn`, `oddity`.`bat` FROM `oddity` AS `oddity`  WHERE `__bar`.`meta` = "foo"  ',
 
-      // The number of queries that will be returned after calling Sequel.select()
+      // The number of queries that will be returned after calling Sequel.find()
       queriesReturned: 1
     }
   }
diff --git a/test/queries/simpleSelectNestedFilterNoAlias.js b/test/queries/simpleSelectNestedFilterNoAlias.js
index 7ca550b..33efbda 100644
--- a/test/queries/simpleSelectNestedFilterNoAlias.js
+++ b/test/queries/simpleSelectNestedFilterNoAlias.js
@@ -31,10 +31,10 @@ module.exports = {
     // Sequel.find()
     find  : {
 
-      // The queryString we expect to be rendered after calling `Sequel.select()`
+      // The queryString we expect to be rendered after calling `Sequel.find()`
       queryString    : 'SELECT `oddity`.`meta`, `oddity`.`id`, `oddity`.`createdAt`, `oddity`.`updatedAt`, `oddity`.`stubborn`, `oddity`.`bat` FROM `oddity` AS `oddity`  WHERE `__bar`.`meta` = "foo"  ',
 
-      // The number of queries that will be returned after calling Sequel.select()
+      // The number of queries that will be returned after calling Sequel.find()
       queriesReturned: 1
     }
   }
diff --git a/test/queries/simpleSelectNestedFilters.js b/test/queries/simpleSelectNestedFilters.js
index 0edba07..de5ee42 100644
--- a/test/queries/simpleSelectNestedFilters.js
+++ b/test/queries/simpleSelectNestedFilters.js
@@ -55,10 +55,10 @@ module.exports = {
     // Sequel.find()
     find  : {
 
-      // The queryString we expect to be rendered after calling `Sequel.select()`
+      // The queryString we expect to be rendered after calling `Sequel.find()`
       queryString    : 'SELECT `foo`.`color`, `foo`.`id`, `foo`.`createdAt`, `foo`.`updatedAt`, `foo`.`bar`, `foo`.`bat`, `foo`.`baz`, `__bat`.`color_g` AS `bat___color_g`, `__bat`.`color_h` AS `bat___color_h`, `__bat`.`color_i` AS `bat___color_i`, `__bat`.`id` AS `bat___id`, `__bat`.`createdAt` AS `bat___createdAt`, `__bat`.`updatedAt` AS `bat___updatedAt` FROM `foo` AS `foo`  LEFT OUTER JOIN `bat` AS `__bat` ON `foo`.`bat` = `__bat`.`id` WHERE `__bat`.`color_g` = "yellow"  AND LOWER(`foo`.`color`) = "red" ',
 
-      // The number of queries that will be returned after calling Sequel.select()
+      // The number of queries that will be returned after calling Sequel.find()
       queriesReturned: 1
     }
   }
diff --git a/test/queries/simpleSelectObjectFilters.js b/test/queries/simpleSelectObjectFilters.js
index 497e971..e2dffda 100644
--- a/test/queries/simpleSelectObjectFilters.js
+++ b/test/queries/simpleSelectObjectFilters.js
@@ -29,10 +29,10 @@ module.exports = {
     // Sequel.find()
     find  : {
 
-      // The queryString we expect to be rendered after calling `Sequel.select()`
+      // The queryString we expect to be rendered after calling `Sequel.find()`
       queryString    : 'SELECT `foo`.`color`, `foo`.`id`, `foo`.`createdAt`, `foo`.`updatedAt`, `foo`.`bar`, `foo`.`bat`, `foo`.`baz` FROM `foo` AS `foo`  WHERE `foo`.`bat` > 5  ',
 
-      // The number of queries that will be returned after calling Sequel.select()
+      // The number of queries that will be returned after calling Sequel.find()
       queriesReturned: 1
     }
   }
diff --git a/test/queries/simpleSelectPopulate.js b/test/queries/simpleSelectPopulate.js
index 8cb4470..2d15b0c 100644
--- a/test/queries/simpleSelectPopulate.js
+++ b/test/queries/simpleSelectPopulate.js
@@ -51,10 +51,10 @@ module.exports = {
     // Sequel.find()
     find  : {
 
-      // The queryString we expect to be rendered after calling `Sequel.select()`
+      // The queryString we expect to be rendered after calling `Sequel.find()`
       queryString    : 'SELECT `foo`.`color`, `foo`.`id`, `foo`.`createdAt`, `foo`.`updatedAt`, `foo`.`bar`, `foo`.`bat`, `foo`.`baz`, `__bat`.`color_g` AS `bat___color_g`, `__bat`.`color_h` AS `bat___color_h`, `__bat`.`color_i` AS `bat___color_i`, `__bat`.`id` AS `bat___id`, `__bat`.`createdAt` AS `bat___createdAt`, `__bat`.`updatedAt` AS `bat___updatedAt` FROM `foo` AS `foo`  LEFT OUTER JOIN `bat` AS `__bat` ON `foo`.`bat` = `__bat`.`id` ',
 
-      // The number of queries that will be returned after calling Sequel.select()
+      // The number of queries that will be returned after calling Sequel.find()
       queriesReturned: 1
     }
   }
diff --git a/test/queries/simpleSelectSort.js b/test/queries/simpleSelectSort.js
index f6b2ec7..37adc6d 100644
--- a/test/queries/simpleSelectSort.js
+++ b/test/queries/simpleSelectSort.js
@@ -30,10 +30,10 @@ module.exports = {
     // Sequel.find()
     find  : {
 
-      // The queryString we expect to be rendered after calling `Sequel.select()`
+      // The queryString we expect to be rendered after calling `Sequel.find()`
       queryString    : 'SELECT `foo`.`color`, `foo`.`id`, `foo`.`createdAt`, `foo`.`updatedAt`, `foo`.`bar`, `foo`.`bat`, `foo`.`baz` FROM `foo` AS `foo`   ORDER BY `foo`.`color` ASC',
 
-      // The number of queries that will be returned after calling Sequel.select()
+      // The number of queries that will be returned after calling Sequel.find()
       queriesReturned: 1
     }
   }
diff --git a/test/queries/simpleSelectWhere.js b/test/queries/simpleSelectWhere.js
index 496a077..3e1f36b 100644
--- a/test/queries/simpleSelectWhere.js
+++ b/test/queries/simpleSelectWhere.js
@@ -29,10 +29,10 @@ module.exports = {
     // Sequel.find()
     find  : {
 
-      // The queryString we expect to be rendered after calling `Sequel.select()`
+      // The queryString we expect to be rendered after calling `Sequel.find()`
       queryString    : 'SELECT `foo`.`color`, `foo`.`id`, `foo`.`createdAt`, `foo`.`updatedAt`, `foo`.`bar`, `foo`.`bat`, `foo`.`baz` FROM `foo` AS `foo`  WHERE LOWER(`foo`.`color`) = "blue" ',
 
-      // The number of queries that will be returned after calling Sequel.select()
+      // The number of queries that will be returned after calling Sequel.find()
       queriesReturned: 1
     }
   }
diff --git a/test/queries/skeleton.js b/test/queries/skeleton.js
index 817b9bc..3f64a1c 100644
--- a/test/queries/skeleton.js
+++ b/test/queries/skeleton.js
@@ -19,7 +19,7 @@ module.exports = {
     // Sequel.select()
     select: {
 
-      // The queryString we expect to be rendered after calling `Sequel.select()`
+      // The queryString we expect to be rendered after calling Sequel.select()
       queryString    : 'SELECT `foo`.`color`, `foo`.`id`, `foo`.`createdAt`, `foo`.`updatedAt`, `foo`.`bar`, `foo`.`bat`, `foo`.`baz` FROM `foo` AS `foo` ',
 
       // The number of queries that will be returned after calling Sequel.select()
@@ -29,10 +29,10 @@ module.exports = {
     // Sequel.find()
     find  : {
 
-      // The queryString we expect to be rendered after calling `Sequel.select()`
+      // The queryString we expect to be rendered after calling Sequel.find()
       queryString    : 'SELECT `foo`.`color`, `foo`.`id`, `foo`.`createdAt`, `foo`.`updatedAt`, `foo`.`bar`, `foo`.`bat`, `foo`.`baz` FROM `foo` AS `foo`  ',
 
-      // The number of queries that will be returned after calling Sequel.select()
+      // The number of queries that will be returned after calling Sequel.find()
       queriesReturned: 1
     }
   }