Fix unvalidated redirects (#3252)

DataDog · Jun 23, 2023 · e15366b · e15366b
1 parent 1ec168f
commit e15366b
Show file tree

Hide file tree

Showing 3 changed files with 7 additions and 8 deletions.
diff --git a/packages/dd-trace/src/appsec/iast/analyzers/unvalidated-redirect-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/unvalidated-redirect-analyzer.js
@@ -32,7 +32,7 @@ class UnvalidatedRedirectAnalyzer extends InjectionAnalyzer {
     if (!value) return false
 
     const ranges = getRanges(iastContext, value)
-    return !this._isRefererHeader(ranges)
+    return ranges && ranges.length > 0 && !this._isRefererHeader(ranges)
   }
 
   _isRefererHeader (ranges) {

diff --git a/.../dd-trace/test/appsec/iast/analyzers/unvalidated-redirect-analyzer.express.plugin.spec.js b/.../dd-trace/test/appsec/iast/analyzers/unvalidated-redirect-analyzer.express.plugin.spec.js
@@ -72,6 +72,10 @@ describe('Unvalidated Redirect vulnerability', () => {
           const location = newTaintedString(iastCtx, 'http://user@app.com/', 'pathParam', 'Request')
           res.header('X-test', location)
         }, UNVALIDATED_REDIRECT)
+
+        testThatRequestHasNoVulnerability((req, res) => {
+          redirectFunctions.insecureWithResHeaderMethod('location', 'http://user@app.com/', res)
+        }, UNVALIDATED_REDIRECT)
       })
   })
 })
diff --git a/packages/dd-trace/test/appsec/iast/analyzers/unvalidated-redirect-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/unvalidated-redirect-analyzer.spec.js
@@ -38,7 +38,7 @@ describe('unvalidated-redirect-analyzer', () => {
     },
 
     getRanges: (iastContext, value) => {
-      if (value === NOT_TAINTED_LOCATION) return []
+      if (value === NOT_TAINTED_LOCATION) return null
 
       if (value === TAINTED_HEADER_REFERER_ONLY) {
         return [REFERER_RANGE]
@@ -52,6 +52,7 @@ describe('unvalidated-redirect-analyzer', () => {
 
   let report
   beforeEach(() => {
+    sinon.stub(overheadController, 'hasQuota').returns(1)
     report = sinon.stub(unvalidatedRedirectAnalyzer, '_report')
   })
 
@@ -91,24 +92,18 @@ describe('unvalidated-redirect-analyzer', () => {
   })
 
   it('should report Location header with tainted string value', () => {
-    sinon.stub(overheadController, 'hasQuota').returns(1)
-
     unvalidatedRedirectAnalyzer.analyze('Location', TAINTED_LOCATION)
 
     expect(report).to.be.called
   })
 
   it('should not report if tainted origin is referer header exclusively', () => {
-    sinon.stub(overheadController, 'hasQuota').returns(1)
-
     unvalidatedRedirectAnalyzer.analyze('Location', TAINTED_HEADER_REFERER_ONLY)
 
     expect(report).to.not.be.called
   })
 
   it('should report if tainted origin contains referer header among others', () => {
-    sinon.stub(overheadController, 'hasQuota').returns(1)
-
     unvalidatedRedirectAnalyzer.analyze('Location', TAINTED_HEADER_REFERER_AMONG_OTHERS)
 
     expect(report).to.be.called