Fixing fs and rasp/iast problems

Author: uurien

packages/dd-trace/src/appsec/rasp/fs-plugin.js

@@ -14,25 +14,27 @@ const enabledFor = {
 
 let fsPlugin
 
-function enterWith (fsProps, store = storage('legacy').getStore()) {
+function storeToStart (fsProps, store = storage('legacy').getStore()) {
   if (store && !store.fs?.opExcluded) {
-    storage('legacy').enterWith({
+    return {
       ...store,
       fs: {
         ...store.fs,
         ...fsProps,
         parentStore: store
       }
-    })
+    }
   }
+
+  return store
 }
 
 class AppsecFsPlugin extends Plugin {
   enable () {
-    this.addSub('apm:fs:operation:start', this._onFsOperationStart)
-    this.addSub('apm:fs:operation:finish', this._onFsOperationFinishOrRenderEnd)
-    this.addSub('tracing:datadog:express:response:render:start', this._onResponseRenderStart)
-    this.addSub('tracing:datadog:express:response:render:end', this._onFsOperationFinishOrRenderEnd)
+    this.addBind('apm:fs:operation:start', this._onFsOperationStart)
+    this.addBind('apm:fs:operation:finish', this._onFsOperationFinishOrRenderEnd)
+    this.addBind('tracing:datadog:express:response:render:start', this._onResponseRenderStart)
+    this.addBind('tracing:datadog:express:response:render:end', this._onFsOperationFinishOrRenderEnd)
 
     super.configure(true)
   }
@@ -44,19 +46,20 @@ class AppsecFsPlugin extends Plugin {
   _onFsOperationStart () {
     const store = storage('legacy').getStore()
     if (store) {
-      enterWith({ root: store.fs?.root === undefined }, store)
+      return storeToStart({ root: store.fs?.root === undefined }, store)
     }
   }
 
   _onResponseRenderStart () {
-    enterWith({ opExcluded: true })
+    return storeToStart({ opExcluded: true })
   }
 
   _onFsOperationFinishOrRenderEnd () {
     const store = storage('legacy').getStore()
-    if (store?.fs?.parentStore) {
-      storage('legacy').enterWith(store.fs.parentStore)
+    if (store?.fs) {
+      return store.fs.parentStore
     }
+    return store
   }
 }
 

packages/dd-trace/test/appsec/iast/analyzers/path-traversal-analyzer.spec.js

@@ -477,6 +477,26 @@ prepareTestServerForIast('integration test', (testThatRequestHasVulnerability, t
 
   describe('test stat', () => {
     runFsMethodTestThreeWay('stat', 0, null, __filename)
+
+    describe('with two calls to async method without waiting to the callback', () => {
+      const fsAsyncWayMethodPath = path.join(os.tmpdir(), 'fs-async-way-method.js')
+
+      before(() => {
+        fs.copyFileSync(path.join(__dirname, 'resources', 'fs-async-way-method.js'), fsAsyncWayMethodPath)
+      })
+
+      after(() => {
+        fs.unlinkSync(fsAsyncWayMethodPath)
+      })
+
+      testThatRequestHasVulnerability(function () {
+        const store = storage('legacy').getStore()
+        const iastCtx = iastContextFunctions.getIastContext(store)
+        const callArgs = [fsAsyncWayMethodPath]
+        callArgs[0] = newTaintedString(iastCtx, callArgs[0], 'param', 'Request')
+        return require(fsAsyncWayMethodPath).doubleCallIgnoringCb('stat', callArgs)
+      }, 'PATH_TRAVERSAL', { occurrences: 2 })
+    })
   })
 
   describe('test symlink', () => {

packages/dd-trace/test/appsec/iast/analyzers/resources/fs-async-way-method.js

@@ -2,10 +2,21 @@
 
 const fs = require('fs')
 
-module.exports = function (methodName, args, cb) {
+function main (methodName, args, cb) {
   return new Promise((resolve, reject) => {
     fs[methodName](...args, (err, res) => {
       resolve(cb(res))
     })
   })
 }
+
+main.doubleCallIgnoringCb = function (methodName, args) {
+  return new Promise((resolve) => {
+    fs[methodName](...args, () => {})
+    fs[methodName](...args, () => {
+      resolve()
+    })
+  })
+}
+
+module.exports = main

packages/dd-trace/test/appsec/rasp/lfi.express.plugin.spec.js

@@ -296,6 +296,13 @@ describe('RASP - lfi', () => {
 
       describe('test readFile', () => {
         runFsMethodTestThreeWay('readFile', undefined, __filename)
+
+        runFsMethodTest('an async operation without callback is executed before',
+          { getAppFn: getAppSync, ruleEvalCount: 2 }, (args) => {
+            const fs = require('fs')
+            fs.readFile(path.join(__dirname, 'utils.js'), () => {}) // safe and ignored operation
+            return fs.readFileSync(...args)
+          }, __filename)
       })
 
       describe('test readlink', () => {