Sequelize DB sources working for SQLi

packages/datadog-instrumentations/src/sequelize.js

@@ -13,7 +13,7 @@ addHook({ name: 'sequelize', versions: ['>=4'] }, Sequelize => {
   const finishCh = channel('datadog:sequelize:query:finish')
 
   shimmer.wrap(Sequelize.prototype, 'query', query => {
-    return function (sql) {
+    return function (sql, options) {
       if (!startCh.hasSubscribers) {
         return query.apply(this, arguments)
       }
@@ -27,9 +27,14 @@ addHook({ name: 'sequelize', versions: ['>=4'] }, Sequelize => {
         dialect = this.dialect.name
       }
 
-      function onFinish () {
+      function onFinish (result) {
+        const type = options?.type || 'RAW'
+        if (type === 'RAW' && result && result.length > 1) {
+          result = result[0]
+        }
+
         asyncResource.bind(function () {
-          finishCh.publish()
+          finishCh.publish({ result })
         }, this).apply(this)
       }
 
@@ -40,7 +45,7 @@ addHook({ name: 'sequelize', versions: ['>=4'] }, Sequelize => {
         })
 
         const promise = query.apply(this, arguments)
-        promise.then(onFinish, onFinish)
+        promise.then(onFinish, () => { onFinish() })
 
         return promise
       }, this).apply(this, arguments)

packages/dd-trace/src/appsec/iast/analyzers/code-injection-analyzer.js

@@ -9,7 +9,9 @@ class CodeInjectionAnalyzer extends InjectionAnalyzer {
   }
 
   onConfigure () {
-    this.addSub('datadog:eval:call', ({ script }) => this.analyze(script))
+    this.addSub('datadog:eval:call', ({ script }) => {
+      this.analyze(script)
+    })
   }
 }
 

packages/dd-trace/src/appsec/iast/analyzers/injection-analyzer.js

@@ -1,19 +1,32 @@
 'use strict'
 const Analyzer = require('./vulnerability-analyzer')
 const { isTainted, getRanges } = require('../taint-tracking/operations')
+const { SQL_ROW_VALUE } = require('../taint-tracking/source-types')
 
 class InjectionAnalyzer extends Analyzer {
   _isVulnerable (value, iastContext) {
     if (value) {
-      return isTainted(iastContext, value)
+      if (!isTainted(iastContext, value)) {
+        return false
+      }
+
+      return this._rangesAreExpected(getRanges(iastContext, value))
     }
+
     return false
   }
 
   _getEvidence (value, iastContext) {
     const ranges = getRanges(iastContext, value)
     return { value, ranges }
   }
+
+  _rangesAreExpected (ranges) {
+    if (!ranges) return false
+
+    const nonRowRanges = ranges.filter(range => range.iinfo.type !== SQL_ROW_VALUE)
+    return nonRowRanges.length > 0
+  }
 }
 
 module.exports = InjectionAnalyzer

packages/dd-trace/src/appsec/iast/analyzers/sql-injection-analyzer.js

@@ -82,6 +82,10 @@ class SqlInjectionAnalyzer extends InjectionAnalyzer {
       return knexDialect.toUpperCase()
     }
   }
+
+  _rangesAreExpected () {
+    return true
+  }
 }
 
 module.exports = new SqlInjectionAnalyzer()

packages/dd-trace/src/appsec/iast/taint-tracking/plugin.js

@@ -12,7 +12,8 @@ const {
   HTTP_REQUEST_HEADER_NAME,
   HTTP_REQUEST_PARAMETER,
   HTTP_REQUEST_PATH_PARAM,
-  HTTP_REQUEST_URI
+  HTTP_REQUEST_URI,
+  SQL_ROW_VALUE
 } = require('./source-types')
 const { EXECUTED_SOURCE } = require('../telemetry/iast-metric')
 
@@ -68,6 +69,11 @@ class TaintTrackingPlugin extends SourceIastPlugin {
       ({ cookies }) => this._cookiesTaintTrackingHandler(cookies)
     )
 
+    this.addSub(
+      { channelName: 'datadog:sequelize:query:finish', tag: SQL_ROW_VALUE },
+      ({ result }) => this._taintSequelizeResult(result)
+    )
+
     this.addSub(
       { channelName: 'datadog:express:process_params:start', tag: HTTP_REQUEST_PATH_PARAM },
       ({ req }) => {
@@ -170,6 +176,30 @@ class TaintTrackingPlugin extends SourceIastPlugin {
     this.taintHeaders(req.headers, iastContext)
     this.taintUrl(req, iastContext)
   }
+
+  _taintSequelizeResult (result, iastContext = getIastContext(storage.getStore())) {
+    if (!iastContext) return result
+
+    const rowsToTaint = 1 // TODO fill this from config
+    if (Array.isArray(result)) {
+      for (let i = 0; i < result.length && i < rowsToTaint; i++) {
+        result[i] = this._taintSequelizeResult(result[i], iastContext)
+      }
+    } else if (result && typeof result === 'object') {
+      if (result.dataValues) {
+        result.dataValues = this._taintSequelizeResult(result.dataValues, iastContext)
+      } else {
+        Object.keys(result).forEach(key => {
+          result[key] = this._taintSequelizeResult(result[key], iastContext)
+        })
+      }
+    } else if (typeof result === 'string') {
+      result = newTaintedString(iastContext, result, SQL_ROW_VALUE, SQL_ROW_VALUE)
+      // console.log('tainting?', result, getRanges(iastContext, result))
+    }
+
+    return result
+  }
 }
 
 module.exports = new TaintTrackingPlugin()

packages/dd-trace/src/appsec/iast/taint-tracking/source-types.js

@@ -11,5 +11,6 @@ module.exports = {
   HTTP_REQUEST_PATH_PARAM: 'http.request.path.parameter',
   HTTP_REQUEST_URI: 'http.request.uri',
   KAFKA_MESSAGE_KEY: 'kafka.message.key',
-  KAFKA_MESSAGE_VALUE: 'kafka.message.value'
+  KAFKA_MESSAGE_VALUE: 'kafka.message.value',
+  SQL_ROW_VALUE: 'sql.row.value'
 }

packages/dd-trace/test/appsec/iast/taint-tracking/sources/sql_row.pg.plugin.spec.js

@@ -0,0 +1 @@
+'use strict'

packages/dd-trace/test/appsec/iast/taint-tracking/sources/sql_row.sequelize.plugin.spec.js

@@ -0,0 +1,105 @@
+'use strict'
+
+const { prepareTestServerForIast } = require('../../utils')
+
+describe('db sources with sequelize', () => {
+  withVersions('sequelize', 'sequelize', sequelizeVersion => {
+    prepareTestServerForIast('sequelize', (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => {
+      let Sequelize, sequelize
+
+      beforeEach(async () => {
+        Sequelize = require(`../../../../../../../versions/sequelize@${sequelizeVersion}`).get()
+        sequelize = new Sequelize('database', 'username', 'password', {
+          dialect: 'sqlite',
+          logging: false
+        })
+        await sequelize.query(`CREATE TABLE examples (
+  id INT,
+  name VARCHAR(50),
+  query VARCHAR(100),
+  command VARCHAR(50),
+  createdAt DATETIME DEFAULT CURRENT_TIMESTAMP,
+  updatedAt DATETIME DEFAULT CURRENT_TIMESTAMP )`)
+
+        await sequelize.query(`INSERT INTO examples (id, name, query, command)
+                               VALUES (1, 'Item1', 'SELECT 1', 'ls'),
+                                      (2, 'Item2', 'SELECT 1', 'ls'),
+                                      (3, 'Item3', 'SELECT 1', 'ls')`)
+      })
+
+      afterEach(() => {
+        return sequelize.close()
+      })
+
+      describe('using query method', () => {
+        testThatRequestHasVulnerability(async (req, res) => {
+          const result = await sequelize.query('SELECT * from examples')
+
+          await sequelize.query(result[0][0].query)
+
+          res.end('OK')
+        }, 'SQL_INJECTION', { occurrences: 1 }, null, null,
+        'Should have SQL_INJECTION using the first row of the result')
+
+        testThatRequestHasNoVulnerability(async (req, res) => {
+          const result = await sequelize.query('SELECT * from examples')
+
+          await sequelize.query(result[0][1].query)
+
+          res.end('OK')
+        }, 'SQL_INJECTION', null, 'Should not taint the second row of a query with default configuration')
+
+        testThatRequestHasNoVulnerability(async (req, res) => {
+          const result = await sequelize.query('SELECT * from examples')
+
+          const childProcess = require('child_process')
+          childProcess.execSync(result[0][0].command)
+
+          res.end('OK')
+        }, 'COMMAND_INJECTION', null, 'Should not detect COMMAND_INJECTION with database source')
+      })
+
+      describe('using Model', () => {
+        // let Model
+        let Example
+
+        beforeEach(() => {
+          Example = sequelize.define('example', {
+            id: {
+              type: Sequelize.DataTypes.INTEGER,
+              primaryKey: true
+            },
+            name: Sequelize.DataTypes.STRING,
+            query: Sequelize.DataTypes.STRING,
+            command: Sequelize.DataTypes.STRING
+          })
+        })
+
+        testThatRequestHasVulnerability(async (req, res) => {
+          const examples = await Example.findAll()
+
+          await sequelize.query(examples[0].query)
+
+          res.end('OK')
+        }, 'SQL_INJECTION', { occurrences: 1 }, null, null,
+        'Should have SQL_INJECTION using the first row of the result')
+
+        testThatRequestHasNoVulnerability(async (req, res) => {
+          const examples = await Example.findAll()
+
+          await sequelize.query(examples[1].query)
+
+          res.end('OK')
+        }, 'SQL_INJECTION', null, 'Should not taint the second row of a query with default configuration')
+
+        testThatRequestHasNoVulnerability(async (req, res) => {
+          const examples = await Example.findAll()
+
+          childProcess.execSync(examples[0].command)
+
+          res.end('OK')
+        }, 'COMMAND_INJECTION', null, 'Should not detect COMMAND_INJECTION with database source')
+      })
+    })
+  })
+})

packages/dd-trace/test/appsec/iast/utils.js

@@ -256,8 +256,8 @@ function prepareTestServerForIast (description, tests, iastConfig) {
       })
     }
 
-    function testThatRequestHasNoVulnerability (fn, vulnerability, makeRequest) {
-      it(`should not have ${vulnerability} vulnerability`, function (done) {
+    function testThatRequestHasNoVulnerability (fn, vulnerability, makeRequest, description) {
+      it(description || `should not have ${vulnerability} vulnerability`, function (done) {
         app = fn
         checkNoVulnerabilityInRequest(vulnerability, config, done, makeRequest)
       })

packages/dd-trace/test/plugins/externals.json

@@ -410,6 +410,10 @@
     {
       "name": "express",
       "versions": [">=4"]
+    },
+    {
+      "name": "sqlite3",
+      "versions": ["^5.0.8"]
     }
   ]
 }