From e15366b17a57872cfe3709fc7dede1fb8168a42e Mon Sep 17 00:00:00 2001 From: Ugaitz Urien Date: Fri, 16 Jun 2023 15:55:05 +0200 Subject: [PATCH] Fix unvalidated redirects (#3252) --- .../iast/analyzers/unvalidated-redirect-analyzer.js | 2 +- .../unvalidated-redirect-analyzer.express.plugin.spec.js | 4 ++++ .../iast/analyzers/unvalidated-redirect-analyzer.spec.js | 9 ++------- 3 files changed, 7 insertions(+), 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 index f05144543f3..25f919844b1 100644 --- 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/packages/dd-trace/test/appsec/iast/analyzers/unvalidated-redirect-analyzer.express.plugin.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/unvalidated-redirect-analyzer.express.plugin.spec.js index 238af6d0210..2686f6e2d1b 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/unvalidated-redirect-analyzer.express.plugin.spec.js +++ b/packages/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 index c18f8cbbee5..d8b1d9f15db 100644 --- 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