Merge pull request #18 from strongloop/feature/fix-sql-injection

Fix SQL injection

Author: raymondfeng

lib/mssql.js

@@ -423,6 +423,50 @@ function dateToMsSql(val) {
   }
 }
 
+function escape(val) {
+  if (val === undefined || val === null) {
+    return 'NULL';
+  }
+
+  switch (typeof val) {
+    case 'boolean':
+      return (val) ? 1 : 0;
+    case 'number':
+      return val + '';
+  }
+
+  if (typeof val === 'object') {
+    val = (typeof val.toISOString === 'function')
+      ? val.toISOString()
+      : val.toString();
+  }
+
+  val = val.replace(/[\0\n\r\b\t\\\'\"\x1a]/g, function (s) {
+    switch (s) {
+      case "\0":
+        return "\\0";
+      case "\n":
+        return "\\n";
+      case "\r":
+        return "\\r";
+      case "\b":
+        return "\\b";
+      case "\t":
+        return "\\t";
+      case "\x1a":
+        return "\\Z";
+      case "\'":
+        return "''"; // For sql server, double quote
+      case "\"":
+        return s; // For oracle
+      default:
+        return "\\" + s;
+    }
+  });
+  // return "q'#"+val+"#'";
+  return "'" + val + "'";
+}
+
 //toDatabase is used for formatting data when inserting/updating records
 // it is also used when building a where clause for filtering selects
 //  in the case of update/insert we want the data to be returned raw
@@ -437,7 +481,7 @@ MsSQL.prototype.toDatabase = function (prop, val, wrap) {
     return null;
   }
   if (prop.type && prop.type.modelName) {
-    return "'" + JSON.stringify(val) + "'";
+    return escape(JSON.stringify(val));
   }
   if (val.constructor && val.constructor.name === 'Object') {
     var operator = Object.keys(val)[0]
@@ -456,12 +500,12 @@ MsSQL.prototype.toDatabase = function (prop, val, wrap) {
         //check if it is an array of string, because in that cause we need to
         // wrap them in single quotes
         if (typeof val[0] === 'string') {
-          return "'" + val.join("','") + "'";
+          return escape(val.join("','"));
         }
         return val.join(',');
       } else {
         if (typeof val === 'string') {
-          val = "'" + val + "'";
+          val = escape(val);
         }
         return val;
       }
@@ -471,12 +515,12 @@ MsSQL.prototype.toDatabase = function (prop, val, wrap) {
   }
   if (!prop) {
     if (typeof val === 'string' && wrap) {
-      val = "'" + val + "'";
+      val = escape(val);
     }
     return val;
   }
   if (prop.type.name === 'Number') {
-    return val;
+    return escape(val);
   }
   if (prop.type.name === 'Date') {
     if (!val) {
@@ -488,7 +532,7 @@ MsSQL.prototype.toDatabase = function (prop, val, wrap) {
     }
     val = dateToMsSql(val);
     if (wrap) {
-      val = "'" + val + "'";
+      val = escape(val);
     }
     return val;
   }
@@ -501,7 +545,7 @@ MsSQL.prototype.toDatabase = function (prop, val, wrap) {
   }
 
   if (wrap) {
-    return "'" + val.toString().replace(/'/g, "''") + "'";
+    return escape(val.toString());
   }
   return val.toString();
 };

test/mssql.test.js

@@ -0,0 +1,137 @@
+require('./init');
+
+var should = require('should');
+
+var Post, db;
+
+describe('mssql connector', function () {
+
+  before(function () {
+    db = getDataSource();
+
+    Post = db.define('PostWithBoolean', {
+      title: { type: String, length: 255, index: true },
+      content: { type: String },
+      approved: Boolean
+    });
+  });
+
+  it('should run migration', function (done) {
+    db.automigrate('PostWithBoolean', function () {
+      done();
+    });
+  });
+  
+  var post;
+  it('should support boolean types with true value', function(done) {
+    Post.create({title: 'T1', content: 'C1', approved: true}, function(err, p) {
+      should.not.exists(err);
+      post = p;
+      Post.findById(p.id, function(err, p) {
+        should.not.exists(err);
+        p.should.have.property('approved', true);
+        done();
+      });
+    });
+  });
+
+  it('should support updating boolean types with false value', function(done) {
+    Post.update({id: post.id}, {approved: false}, function(err) {
+      should.not.exists(err);
+      Post.findById(post.id, function(err, p) {
+        should.not.exists(err);
+        p.should.have.property('approved', false);
+        done();
+      });
+    });
+  });
+
+
+  it('should support boolean types with false value', function(done) {
+    Post.create({title: 'T2', content: 'C2', approved: false}, function(err, p) {
+      should.not.exists(err);
+      post = p;
+      Post.findById(p.id, function(err, p) {
+        should.not.exists(err);
+        p.should.have.property('approved', false);
+        done();
+      });
+    });
+  });
+
+  it('should single quote escape', function(done) {
+    Post.create({title: "T2", content: "C,D", approved: false}, function(err, p) {
+      should.not.exists(err);
+      post = p;
+      Post.findById(p.id, function(err, p) {
+        should.not.exists(err);
+        p.should.have.property('content', "C,D");
+        done();
+      });
+    });
+  });
+
+  it('should return the model instance for upsert', function(done) {
+    Post.upsert({id: post.id, title: 'T2_new', content: 'C2_new',
+      approved: true}, function(err, p) {
+      p.should.have.property('id', post.id);
+      p.should.have.property('title', 'T2_new');
+      p.should.have.property('content', 'C2_new');
+      p.should.have.property('approved', true);
+      done();
+    });
+  });
+
+  it('should return the model instance for upsert when id is not present',
+    function(done) {
+      Post.upsert({title: 'T2_new', content: 'C2_new', approved: true},
+        function(err, p) {
+          p.should.have.property('id');
+          p.should.have.property('title', 'T2_new');
+          p.should.have.property('content', 'C2_new');
+          p.should.have.property('approved', true);
+          done();
+        });
+    });
+
+  it('should escape number values to defect SQL injection in findById',
+    function(done) {
+      Post.findById('(SELECT 1+1)', function(err, p) {
+        should.exists(err);
+        done();
+      });
+    });
+
+  it('should escape number values to defect SQL injection in find',
+    function(done) {
+      Post.find({where: {id: '(SELECT 1+1)'}}, function(err, p) {
+        should.exists(err);
+        done();
+      });
+    });
+
+  it('should escape number values to defect SQL injection in find with gt',
+    function(done) {
+      Post.find({where: {id: {gt: '(SELECT 1+1)'}}}, function(err, p) {
+        should.exists(err);
+        done();
+      });
+    });
+
+  it('should escape number values to defect SQL injection in find',
+    function(done) {
+      Post.find({limit: '(SELECT 1+1)'}, function(err, p) {
+        should.exists(err);
+        done();
+      });
+    });
+
+  it('should escape number values to defect SQL injection in find with inq',
+    function(done) {
+      Post.find({where: {id: {inq: ['(SELECT 1+1)']}}}, function(err, p) {
+        should.exists(err);
+        done();
+      });
+    });
+
+});