From e5ba4729f62dd01db6ca9c53a6370cae719113c0 Mon Sep 17 00:00:00 2001 From: zhoulin Date: Fri, 10 Jul 2020 17:23:29 +0800 Subject: [PATCH 1/4] feat: csrf support check origin header with referer type --- app/extend/context.js | 16 +++---- test/csrf.test.js | 98 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 95 insertions(+), 19 deletions(-) diff --git a/app/extend/context.js b/app/extend/context.js index c7e6a01..e35b167 100644 --- a/app/extend/context.js +++ b/app/extend/context.js @@ -203,19 +203,21 @@ module.exports = { [CSRF_REFERER_CHECK]() { const { refererWhiteList } = this.app.config.security.csrf; - const referer = (this.headers.referer || '').toLowerCase(); + // check Origin/Referer headers + const referer = (this.headers.referer || this.headers.origin || '').toLowerCase(); + if (!referer) { - debug('missing csrf referer'); - this[LOG_CSRF_NOTICE]('missing csrf referer'); - return 'missing csrf referer'; + debug('missing csrf referer or origin'); + this[LOG_CSRF_NOTICE]('missing csrf referer or origin'); + return 'missing csrf referer or origin'; } const host = utils.getFromUrl(referer, 'host'); const domainList = refererWhiteList.concat(this.host); if (!host || !utils.isSafeDomain(host, domainList)) { - debug('verify referer error'); - this[LOG_CSRF_NOTICE]('invalid csrf referer'); - return 'invalid csrf referer'; + debug('verify referer or origin error'); + this[LOG_CSRF_NOTICE]('invalid csrf referer or origin'); + return 'invalid csrf referer or origin'; } }, diff --git a/test/csrf.test.js b/test/csrf.test.js index ffe2552..ad36a2d 100644 --- a/test/csrf.test.js +++ b/test/csrf.test.js @@ -469,7 +469,7 @@ describe('test/csrf.test.js', () => { } }); - it('should return 200 with correct referer when type is referer', function* () { + it('should return 200 with correct referer or origin when type is referer', function* () { mm(this.app.config, 'env', 'local'); mm(this.app.config.security.csrf, 'type', 'referer'); mm(this.app.config.security.csrf, 'refererWhiteList', [ '.nodejs.org' ]); @@ -479,9 +479,14 @@ describe('test/csrf.test.js', () => { .set('accept', 'text/html') .set('referer', 'https://nodejs.org/en/') .expect(200); + yield this.app.httpRequest() + .post('/update') + .set('accept', 'text/html') + .set('origin', 'https://nodejs.org/en/') + .expect(200); }); - it('should return 403 with correct referer when type is referer', function* () { + it('should return 403 with correct referer or origin when type is referer', function* () { mm(this.app.config, 'env', 'local'); mm(this.app.config.security.csrf, 'type', 'referer'); mm(this.app.config.security.csrf, 'refererWhiteList', [ 'nodejs.org' ]); @@ -491,6 +496,12 @@ describe('test/csrf.test.js', () => { .set('accept', 'text/html') .set('referer', 'https://wwwnodejs.org/en/') .expect(403); + + yield this.app.httpRequest() + .post('/update') + .set('accept', 'text/html') + .set('origin', 'https://wwwnodejs.org/en/') + .expect(403); }); it('should return 200 with same root host when type is referer', function* () { @@ -509,6 +520,19 @@ describe('test/csrf.test.js', () => { .set('referer', 'https://nodejs.org/en/') .set('host', 'nodejs.org') .expect(200); + + yield this.app.httpRequest() + .post('/update') + .set('accept', 'text/html') + .set('origin', 'https://www.nodejs.org/en/') + .set('host', 'nodejs.org') + .expect(200); + yield this.app.httpRequest() + .post('/update') + .set('accept', 'text/html') + .set('origin', 'https://nodejs.org/en/') + .set('host', 'nodejs.org') + .expect(200); }); it('should return 403 with invalid host when type is referer', function* () { @@ -521,9 +545,16 @@ describe('test/csrf.test.js', () => { .set('referer', 'https://wwwnodejs.org/en/') .set('host', 'nodejs.org') .expect(403); + + yield this.app.httpRequest() + .post('/update') + .set('accept', 'text/html') + .set('origin', 'https://wwwnodejs.org/en/') + .set('host', 'nodejs.org') + .expect(403); }); - it('should return 403 with evil referer when type is referer', function* () { + it('should return 403 with evil referer or origin when type is referer', function* () { mm(this.app.config, 'env', 'local'); mm(this.app.config.security.csrf, 'type', 'referer'); mm(this.app.config.security.csrf, 'refererWhiteList', [ 'nodejs.org' ]); @@ -533,9 +564,14 @@ describe('test/csrf.test.js', () => { .set('accept', 'text/html') .set('referer', 'https://nodejs.org!.evil.com/en/') .expect(403); + yield this.app.httpRequest() + .post('/update') + .set('accept', 'text/html') + .set('origin', 'https://nodejs.org!.evil.com/en/') + .expect(403); }); - it('should return 403 with illegal referer when type is referer', function* () { + it('should return 403 with illegal referer or origin when type is referer', function* () { mm(this.app.config, 'env', 'local'); mm(this.app.config.security.csrf, 'type', 'referer'); mm(this.app.config.security.csrf, 'refererWhiteList', [ 'nodejs.org' ]); @@ -545,6 +581,11 @@ describe('test/csrf.test.js', () => { .set('accept', 'text/html') .set('referer', '/en/') .expect(403); + yield this.app.httpRequest() + .post('/update') + .set('accept', 'text/html') + .set('origin', '/en/') + .expect(403); }); it('should return 200 with same domain request', function* () { @@ -557,6 +598,13 @@ describe('test/csrf.test.js', () => { .set('accept', 'text/html') .set('referer', `http://127.0.0.1:${port}/`) .expect(200); + + const httpRequestObj2 = this.app.httpRequest().post('/update'); + const port2 = httpRequestObj2.app.address().port; + yield httpRequestObj2 + .set('accept', 'text/html') + .set('origin', `http://127.0.0.1:${port2}/`) + .expect(200); }); it('should return 403 with different domain request', function* () { @@ -568,7 +616,14 @@ describe('test/csrf.test.js', () => { .set('accept', 'text/html') .set('referer', 'https://nodejs.org/en/') .expect(403) - .expect(/invalid csrf referer/); + .expect(/invalid csrf referer or origin/); + + yield this.app.httpRequest() + .post('/update') + .set('accept', 'text/html') + .set('origin', 'https://nodejs.org/en/') + .expect(403) + .expect(/invalid csrf referer or origin/); }); it('should check both ctoken and referer when type is all', function* () { @@ -581,13 +636,19 @@ describe('test/csrf.test.js', () => { .set('referer', 'https://eggjs.org/en/') .expect(403) .expect(/missing csrf token/); + yield this.app.httpRequest() + .post('/update') + .set('accept', 'text/html') + .set('origin', 'https://eggjs.org/en/') + .expect(403) + .expect(/missing csrf token/); yield this.app.httpRequest() .post('/update') .send({ _csrf: '1' }) .set('accept', 'text/html') .set('cookie', 'csrfToken=1') .expect(403) - .expect(/missing csrf referer/); + .expect(/missing csrf referer or origin/); }); it('should check one of ctoken and referer when type is any', function* () { @@ -599,6 +660,11 @@ describe('test/csrf.test.js', () => { .set('accept', 'text/html') .set('referer', 'https://eggjs.org/en/') .expect(200); + yield this.app.httpRequest() + .post('/update') + .set('accept', 'text/html') + .set('origin', 'https://eggjs.org/en/') + .expect(200); yield this.app.httpRequest() .post('/update') .send({ _csrf: '1' }) @@ -615,7 +681,7 @@ describe('test/csrf.test.js', () => { .expect(/ForbiddenError: both ctoken and referer check error: invalid csrf token, missing csrf referer/); }); - it('should return 403 without referer when type is referer', function* () { + it('should return 403 without referer and origin when type is referer', function* () { mm(this.app.config, 'env', 'local'); mm(this.app.config.security.csrf, 'type', 'referer'); mm(this.app.config.security.csrf, 'refererWhiteList', [ 'https://eggjs.org/' ]); @@ -624,11 +690,11 @@ describe('test/csrf.test.js', () => { .post('/update') .set('accept', 'text/html') .expect(403) - .expect(/missing csrf referer/); - this.app.expectLog('missing csrf referer. See http'); + .expect(/missing csrf referer or origin/); + this.app.expectLog('missing csrf referer or origin. See http'); }); - it('should return 403 with invalid referer when type is referer', function* () { + it('should return 403 with invalid referer or origin when type is referer', function* () { mm(this.app.config, 'env', 'local'); mm(this.app.config.security.csrf, 'type', 'referer'); mm(this.app.config.security.csrf, 'refererWhiteList', [ 'https://eggjs.org/' ]); @@ -638,8 +704,16 @@ describe('test/csrf.test.js', () => { .set('accept', 'text/html') .set('referer', 'https://nodejs.org/en/') .expect(403) - .expect(/invalid csrf referer/); - this.app.expectLog('invalid csrf referer. See http'); + .expect(/invalid csrf referer or origin/); + this.app.expectLog('invalid csrf referer or origin. See http'); + + yield this.app.httpRequest() + .post('/update') + .set('accept', 'text/html') + .set('referer', 'https://nodejs.org/en/') + .expect(403) + .expect(/invalid csrf referer or origin/); + this.app.expectLog('invalid csrf referer or origin. See http'); }); it('should throw with error type', function* () { From aff0e8d276230a643027592de2d48921b460e24d Mon Sep 17 00:00:00 2001 From: zhoulin Date: Fri, 10 Jul 2020 17:56:13 +0800 Subject: [PATCH 2/4] chore: first check origin, if not set then check referer --- app/extend/context.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/extend/context.js b/app/extend/context.js index e35b167..4ab6ff3 100644 --- a/app/extend/context.js +++ b/app/extend/context.js @@ -204,7 +204,7 @@ module.exports = { [CSRF_REFERER_CHECK]() { const { refererWhiteList } = this.app.config.security.csrf; // check Origin/Referer headers - const referer = (this.headers.referer || this.headers.origin || '').toLowerCase(); + const referer = (this.headers.origin || this.headers.referer || '').toLowerCase(); if (!referer) { debug('missing csrf referer or origin'); From 51d02238612467b5ee5b1a3661ce0d59e8cd5449 Mon Sep 17 00:00:00 2001 From: zhoulin Date: Sat, 11 Jul 2020 09:55:09 +0800 Subject: [PATCH 3/4] chore: first check referer for better compatibility --- app/extend/context.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/extend/context.js b/app/extend/context.js index 4ab6ff3..e35b167 100644 --- a/app/extend/context.js +++ b/app/extend/context.js @@ -204,7 +204,7 @@ module.exports = { [CSRF_REFERER_CHECK]() { const { refererWhiteList } = this.app.config.security.csrf; // check Origin/Referer headers - const referer = (this.headers.origin || this.headers.referer || '').toLowerCase(); + const referer = (this.headers.referer || this.headers.origin || '').toLowerCase(); if (!referer) { debug('missing csrf referer or origin'); From e618d53e543c62d225f3e85f0916f688787ee4a5 Mon Sep 17 00:00:00 2001 From: fengmk2 Date: Mon, 13 Jan 2025 20:11:21 +0800 Subject: [PATCH 4/4] Update csrf.test.js --- test/csrf.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/csrf.test.js b/test/csrf.test.js index 2dc7681..912fed9 100644 --- a/test/csrf.test.js +++ b/test/csrf.test.js @@ -547,7 +547,7 @@ describe('test/csrf.test.js', () => { .set('host', 'nodejs.org') .expect(403); - yield this.app.httpRequest() + await app.httpRequest() .post('/update') .set('accept', 'text/html') .set('origin', 'https://wwwnodejs.org/en/')