[Aikido] Prevent NoSQL injection in login handler by validating string types

package.json.new

@@ -0,0 +1,58 @@
+{
+  "name": "goof",
+  "version": "1.0.1",
+  "description": "A vulnerable todo demo application",
+  "homepage": "https://snyk.io/",
+  "repository": {
+    "type": "git",
+    "url": "https://github.com/Snyk/snyk-todo-list-demo-app/"
+  },
+  "scripts": {
+    "dev": "NODE_OPTIONS=--openssl-legacy-provider nodemon ./app.js",
+    "start": "NODE_OPTIONS=--openssl-legacy-provider node app.js",
+    "build": "browserify -r jquery > public/js/bundle.js",
+    "cleanup": "mongo express-todo --eval 'db.todos.remove({});'",
+    "test": "tap tests/*.test.js",
+    "test:security": "snyk test"
+  },
+  "dependencies": {
+    "adm-zip": "0.4.11",
+    "body-parser": "1.9.0",
+    "cfenv": "^1.0.4",
+    "consolidate": "0.14.5",
+    "dustjs-helpers": "1.5.0",
+    "dustjs-linkedin": "2.5.0",
+    "ejs": "1.0.0",
+    "ejs-locals": "1.0.2",
+    "errorhandler": "1.2.0",
+    "express": "4.12.4",
+    "express-fileupload": "0.0.5",
+    "express-session": "^1.17.2",
+    "file-type": "^8.1.0",
+    "hbs": "^4.0.4",
+    "humanize-ms": "1.0.1",
+    "jquery": "^2.2.4",
+    "lodash": "4.17.4",
+    "marked": "0.3.5",
+    "method-override": "latest",
+    "moment": "2.15.1",
+    "mongodb": "^3.5.9",
+    "mongoose": "4.2.4",
+    "morgan": "latest",
+    "ms": "^0.7.1",
+    "mysql": "^2.18.1",
+    "npmconf": "0.0.24",
+    "optional": "^0.1.3",
+    "st": "0.2.4",
+    "stream-buffers": "^3.0.1",
+    "tap": "^21.7.5",
+    "typeorm": "^0.2.25",
+    "validator": "^13.5.2"
+  },
+  "devDependencies": {
+    "browserify": "^13.1.1",
+    "nodemon": "^2.0.7",
+    "snyk": "^1.244.0"
+  },
+  "license": "Apache-2.0"
+}

routes/index.js

@@ -35,6 +35,11 @@
 };
 
 exports.loginHandler = function (req, res, next) {
+  // Validate that username and password are strings to prevent NoSQL injection
+  if (typeof req.body.username !== 'string' || typeof req.body.password !== 'string') {
+    return res.status(401).send()
+  }
+
   if (validator.isEmail(req.body.username)) {
     User.find({ username: req.body.username, password: req.body.password }, function (err, users) {
       if (users.length > 0) {

tests/nosql-injection-validation.test.js

@@ -0,0 +1,153 @@
+const tap = require('tap');
+
+// Test the type validation logic that prevents NoSQL injection
+// This tests the mitigation added to routes/index.js loginHandler
+
+tap.test('NoSQL Injection Prevention - Type Validation Tests', (t) => {
+
+  // Helper function to test type validation
+  function testTypeValidation(username, password) {
+    // This replicates the validation logic from routes/index.js lines 38-41
+    if (typeof username !== 'string' || typeof password !== 'string') {
+      return { valid: false, statusCode: 401 };
+    }
+    return { valid: true };
+  }
+
+  t.test('should reject password as MongoDB $ne operator object', (t) => {
+    const result = testTypeValidation('admin@snyk.io', { '$ne': null });
+    t.equal(result.valid, false, 'should reject object password');
+    t.equal(result.statusCode, 401, 'should return 401');
+    t.end();
+  });
+
+  t.test('should reject password as MongoDB $gt operator object', (t) => {
+    const result = testTypeValidation('admin@snyk.io', { '$gt': '' });
+    t.equal(result.valid, false, 'should reject object password');
+    t.equal(result.statusCode, 401, 'should return 401');
+    t.end();
+  });
+
+  t.test('should reject password as MongoDB $regex operator object', (t) => {
+    const result = testTypeValidation('admin@snyk.io', { '$regex': '.*' });
+    t.equal(result.valid, false, 'should reject object password');
+    t.equal(result.statusCode, 401, 'should return 401');
+    t.end();
+  });
+
+  t.test('should reject password as array', (t) => {
+    const result = testTypeValidation('admin@snyk.io', ['password1', 'password2']);
+    t.equal(result.valid, false, 'should reject array password');
+    t.equal(result.statusCode, 401, 'should return 401');
+    t.end();
+  });
+
+  t.test('should reject password as null', (t) => {
+    const result = testTypeValidation('admin@snyk.io', null);
+    t.equal(result.valid, false, 'should reject null password');
+    t.equal(result.statusCode, 401, 'should return 401');
+    t.end();
+  });
+
+  t.test('should reject password as undefined', (t) => {
+    const result = testTypeValidation('admin@snyk.io', undefined);
+    t.equal(result.valid, false, 'should reject undefined password');
+    t.equal(result.statusCode, 401, 'should return 401');
+    t.end();
+  });
+
+  t.test('should reject password as number', (t) => {
+    const result = testTypeValidation('admin@snyk.io', 12345);
+    t.equal(result.valid, false, 'should reject number password');
+    t.equal(result.statusCode, 401, 'should return 401');
+    t.end();
+  });
+
+  t.test('should reject password as boolean', (t) => {
+    const result = testTypeValidation('admin@snyk.io', true);
+    t.equal(result.valid, false, 'should reject boolean password');
+    t.equal(result.statusCode, 401, 'should return 401');
+    t.end();
+  });
+
+  t.test('should reject username as MongoDB operator object', (t) => {
+    const result = testTypeValidation({ '$ne': null }, 'password123');
+    t.equal(result.valid, false, 'should reject object username');
+    t.equal(result.statusCode, 401, 'should return 401');
+    t.end();
+  });
+
+  t.test('should reject username as array', (t) => {
+    const result = testTypeValidation(['admin@snyk.io'], 'password123');
+    t.equal(result.valid, false, 'should reject array username');
+    t.equal(result.statusCode, 401, 'should return 401');
+    t.end();
+  });
+
+  t.test('should reject username as null', (t) => {
+    const result = testTypeValidation(null, 'password123');
+    t.equal(result.valid, false, 'should reject null username');
+    t.equal(result.statusCode, 401, 'should return 401');
+    t.end();
+  });
+
+  t.test('should reject username as undefined', (t) => {
+    const result = testTypeValidation(undefined, 'password123');
+    t.equal(result.valid, false, 'should reject undefined username');
+    t.equal(result.statusCode, 401, 'should return 401');
+    t.end();
+  });
+
+  t.test('should reject both username and password as objects', (t) => {
+    const result = testTypeValidation({ '$ne': null }, { '$ne': null });
+    t.equal(result.valid, false, 'should reject both as objects');
+    t.equal(result.statusCode, 401, 'should return 401');
+    t.end();
+  });
+
+  t.test('should reject nested MongoDB operators in password', (t) => {
+    const result = testTypeValidation('admin@snyk.io', { '$or': [{ '$ne': null }, { '$gt': '' }] });
+    t.equal(result.valid, false, 'should reject nested operators');
+    t.equal(result.statusCode, 401, 'should return 401');
+    t.end();
+  });
+
+  t.test('should reject empty object as password', (t) => {
+    const result = testTypeValidation('admin@snyk.io', {});
+    t.equal(result.valid, false, 'should reject empty object');
+    t.equal(result.statusCode, 401, 'should return 401');
+    t.end();
+  });
+
+  t.test('should accept valid string username and password', (t) => {
+    const result = testTypeValidation('admin@snyk.io', 'somepassword');
+    t.equal(result.valid, true, 'should accept string types');
+    t.notOk(result.statusCode, 'should not set error status code');
+    t.end();
+  });
+
+  t.test('should accept empty string as password (valid string type)', (t) => {
+    const result = testTypeValidation('admin@snyk.io', '');
+    t.equal(result.valid, true, 'should accept empty string');
+    t.notOk(result.statusCode, 'should not set error status code');
+    t.end();
+  });
+
+  t.test('should accept empty string as username (valid string type)', (t) => {
+    const result = testTypeValidation('', 'password123');
+    t.equal(result.valid, true, 'should accept empty string');
+    t.notOk(result.statusCode, 'should not set error status code');
+    t.end();
+  });
+
+  t.test('EXPLOIT SCENARIO: should prevent the original pentest exploit', (t) => {
+    // This is the exact exploit from the pentest finding:
+    // POST /login with JSON body: { "username": "admin@snyk.io", "password": { "$ne": null } }
+    const result = testTypeValidation('admin@snyk.io', { '$ne': null });
+    t.equal(result.valid, false, 'should block the exploit');
+    t.equal(result.statusCode, 401, 'should return 401 unauthorized');
+    t.end();
+  });
+
+  t.end();
+});