From 8495195513f894c06cf4cdd96e151c201a77bb53 Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Tue, 15 Feb 2022 23:10:51 +0100 Subject: [PATCH 01/21] feat: poc of determineRequestReferrer --- lib/fetch/util.js | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index 8d3ddb2282a..316acc3a312 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -333,6 +333,44 @@ function clonePolicyContainer () { // https://w3c.github.io/webappsec-referrer-policy/#determine-requests-referrer function determineRequestsReferrer (request) { // TODO + const policy = request.referrerPolicy + const environment = request.client + let referrerSource = null + + if (request.referrer instanceof URL) { + referrerSource = request.referrer + } + + if (referrerSource === null) return 'no-referrer' + + const urlProtocol = referrerSource.protocol + + if (urlProtocol === 'about:' || urlProtocol === 'data:' || urlProtocol === 'data:') { + return 'no-referrer' + } + + let temp + let referrerOrigin + const referrerUrl = (temp = stripURLForReferrer(referrerSource)).length > 4096 + ? referrerOrigin = stripURLForReferrer(referrerSource, true) + : temp + + // TODO: return accordingly to final switch statement + + function stripURLForReferrer (url, originOnly = false) { + const urlObject = new URL(url.href) + urlObject.username = '' + urlObject.password = '' + urlObject.hash = '' + + if (originOnly) { + urlObject.pathname = '' + urlObject.search = '' + } + + return urlObject.href + } + return 'no-referrer' } From 8472f00590e225e9cb31b2ca72dce137b85351f8 Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Sat, 19 Feb 2022 11:53:39 +0100 Subject: [PATCH 02/21] refactor: apply shortcut --- lib/fetch/util.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index 316acc3a312..5949f71b695 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -334,6 +334,9 @@ function clonePolicyContainer () { function determineRequestsReferrer (request) { // TODO const policy = request.referrerPolicy + + if (policy === '' || policy === 'no-referrer') return 'no-referrer' + const environment = request.client let referrerSource = null From 10cb439601caf649c8aecb6bc12cb268bd528605 Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Sat, 19 Feb 2022 11:54:03 +0100 Subject: [PATCH 03/21] feat(partial): apply switch referrer statement --- lib/fetch/util.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index 5949f71b695..cd455e6b46e 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -357,8 +357,23 @@ function determineRequestsReferrer (request) { const referrerUrl = (temp = stripURLForReferrer(referrerSource)).length > 4096 ? referrerOrigin = stripURLForReferrer(referrerSource, true) : temp + const areSameOrigin = sameOrigin(request, referrerUrl) + // NOTE: how shall we treat step 7? // TODO: return accordingly to final switch statement + switch (policy) { + case 'origin': return referrerOrigin + case 'unsafe-url': return referrerUrl + // TODO: assess if trustworthy or not + case 'strict-origin': return referrerOrigin + case 'strict-origin-when-cross-origin': + return areSameOrigin ? referrerOrigin : 'no-referrer' + case 'origin-when-cross-origin': + return areSameOrigin ? referrerUrl : referrerOrigin + // TODO: assess if trustworthy or not + case 'no-referrer-when-downgrade': + return referrerUrl + } function stripURLForReferrer (url, originOnly = false) { const urlObject = new URL(url.href) From 31cdc42c084ae281f83c878519ca11adf1cb0722 Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Sat, 19 Feb 2022 12:03:19 +0100 Subject: [PATCH 04/21] refactor: add in-code documentation --- lib/fetch/util.js | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index cd455e6b46e..3035a197f3b 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -333,34 +333,54 @@ function clonePolicyContainer () { // https://w3c.github.io/webappsec-referrer-policy/#determine-requests-referrer function determineRequestsReferrer (request) { // TODO + // 1. Let policy be request's referrer policy. const policy = request.referrerPolicy if (policy === '' || policy === 'no-referrer') return 'no-referrer' + // 2. Let environment be the request client const environment = request.client let referrerSource = null + // TODO + // 3(a) If policy is "origin", then + // 3(a)(I) If environment's global object is Window then + + // 3(a)(II) If environment's global object is not Window, + // Let referrerSource be environments creationURL + + // 3(b) If requests's referrer is a URL instance, then make + // referrerSource be requests's referrer. if (request.referrer instanceof URL) { referrerSource = request.referrer } + // If referrerSource is null, then return "no-referrer". if (referrerSource === null) return 'no-referrer' const urlProtocol = referrerSource.protocol + // If url's scheme is a local scheme (i.e. one of "about", "data", "javascript", "file") + // then return "no-referrer". if (urlProtocol === 'about:' || urlProtocol === 'data:' || urlProtocol === 'data:') { return 'no-referrer' } let temp let referrerOrigin - const referrerUrl = (temp = stripURLForReferrer(referrerSource)).length > 4096 + // 4. Let requests's referrerURL be the result of stripping referrer + // source for use as referrer (using util function, without origin only) + const referrerUrl = (temp = stripURLForReferrer(referrerSource)).length > 4096 + // 5. Let referrerOrigin be the result of stripping referrer + // source for use as referrer (using util function, with originOnly true) + // 6. If result of seralizing referrerUrl is a string whose length is greater than + // 4096, then set referrerURL to referrerOrigin ? referrerOrigin = stripURLForReferrer(referrerSource, true) : temp const areSameOrigin = sameOrigin(request, referrerUrl) // NOTE: how shall we treat step 7? - // TODO: return accordingly to final switch statement + // 8. Execute the switch statements corresponding to the value of policy: switch (policy) { case 'origin': return referrerOrigin case 'unsafe-url': return referrerUrl From d5add52004b1c56afe2ba7187f4d97e321ff4dc4 Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Mon, 21 Feb 2022 00:22:07 +0100 Subject: [PATCH 05/21] feat: add check for window --- lib/fetch/util.js | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index 3035a197f3b..6cf9e15bc1c 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -345,19 +345,30 @@ function determineRequestsReferrer (request) { // TODO // 3(a) If policy is "origin", then // 3(a)(I) If environment's global object is Window then + if (request.referrer === 'client') { + if (environment.globalObject instanceof Window) { + const origin = environment.globalObject.self?.origin ?? environment.globalObject.location?.origin + + if (origin == null || origin === 'null') return 'no-referrer' + referrerSource = new URL(environment.globalObject.location.href) + } else { + // 3(a)(II) If environment's global object is not Window, + // Let referrerSource be environments creationURL + if (environment.globalObject.location == null) { + return 'no-referrer' + } - // 3(a)(II) If environment's global object is not Window, - // Let referrerSource be environments creationURL - - // 3(b) If requests's referrer is a URL instance, then make - // referrerSource be requests's referrer. - if (request.referrer instanceof URL) { + referrerSource = new URL(environment.globalObject.location.href) + } + } else if (request.referrer instanceof URL) { + // 3(b) If requests's referrer is a URL instance, then make + // referrerSource be requests's referrer. referrerSource = request.referrer + } else { + // If referrerSource is null, then return "no-referrer". + return 'no-referrer' } - // If referrerSource is null, then return "no-referrer". - if (referrerSource === null) return 'no-referrer' - const urlProtocol = referrerSource.protocol // If url's scheme is a local scheme (i.e. one of "about", "data", "javascript", "file") From b9f50972dc0d9ab4852d67f67397344656649a23 Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Tue, 22 Feb 2022 23:16:26 +0100 Subject: [PATCH 06/21] docs: add comments --- lib/fetch/util.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index 6cf9e15bc1c..22fe185448c 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -336,6 +336,7 @@ function determineRequestsReferrer (request) { // 1. Let policy be request's referrer policy. const policy = request.referrerPolicy + // Return no-referrer when empty or policy says so if (policy === '' || policy === 'no-referrer') return 'no-referrer' // 2. Let environment be the request client @@ -390,7 +391,7 @@ function determineRequestsReferrer (request) { : temp const areSameOrigin = sameOrigin(request, referrerUrl) - // NOTE: how shall we treat step 7? + // NOTE: How to treat step 7? // 8. Execute the switch statements corresponding to the value of policy: switch (policy) { case 'origin': return referrerOrigin @@ -401,7 +402,6 @@ function determineRequestsReferrer (request) { return areSameOrigin ? referrerOrigin : 'no-referrer' case 'origin-when-cross-origin': return areSameOrigin ? referrerUrl : referrerOrigin - // TODO: assess if trustworthy or not case 'no-referrer-when-downgrade': return referrerUrl } From a8b310618876cc89f987ab021127d1aa19c6b3f5 Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Tue, 22 Feb 2022 23:16:52 +0100 Subject: [PATCH 07/21] feat: add check for trustworthy/non-trustworthy urls --- lib/fetch/util.js | 66 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 62 insertions(+), 4 deletions(-) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index 22fe185448c..c841b3c08d0 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -390,20 +390,44 @@ function determineRequestsReferrer (request) { ? referrerOrigin = stripURLForReferrer(referrerSource, true) : temp const areSameOrigin = sameOrigin(request, referrerUrl) + const isNonPotentiallyTrustWorthy = isURLPotentiallyTrustworthy(referrerUrl) && + !isURLPotentiallyTrustworthy(request.url) // NOTE: How to treat step 7? // 8. Execute the switch statements corresponding to the value of policy: switch (policy) { case 'origin': return referrerOrigin case 'unsafe-url': return referrerUrl - // TODO: assess if trustworthy or not - case 'strict-origin': return referrerOrigin + case 'strict-origin': + /** + * 1. If referrerURL is a potentially trustworthy URL and + * request’s current URL is not a potentially trustworthy URL, + * then return no referrer. + * 2. Return referrerOrigin + */ + return isNonPotentiallyTrustWorthy ? 'no-referrer' : referrerOrigin case 'strict-origin-when-cross-origin': + /** + * 1. If the origin of referrerURL and the origin of request’s current URL are the same, + * then return referrerURL. + * 2. If referrerURL is a potentially trustworthy URL and request’s current URL is not a + * potentially trustworthy URL, then return no referrer. + * 3. Return referrerOrigin + */ + if (areSameOrigin) return referrerOrigin + else return isNonPotentiallyTrustWorthy ? 'no-referrer' : referrerOrigin + case 'same-origin': return areSameOrigin ? referrerOrigin : 'no-referrer' case 'origin-when-cross-origin': return areSameOrigin ? referrerUrl : referrerOrigin case 'no-referrer-when-downgrade': - return referrerUrl + /** + * 1. If referrerURL is a potentially trustworthy URL and + * request’s current URL is not a potentially trustworthy URL, + * then return no referrer. + * 2. Return referrerOrigin + */ + return isNonPotentiallyTrustWorthy ? 'no-referrer' : referrerOrigin } function stripURLForReferrer (url, originOnly = false) { @@ -419,8 +443,42 @@ function determineRequestsReferrer (request) { return urlObject.href } +} + +function isURLPotentiallyTrustworthy (url) { + if (!(url instanceof URL)) { + return false + } + + if (url.href === 'about:blank' || url.href === 'about:srcdoc') { + return true + } + + if (url.protocol === 'data:') return true + + return isOriginPotentiallyTrustworthy(url.origin) + + function isOriginPotentiallyTrustworthy(origin) { + if (origin == null || origin === 'null') return false + + let originAsURL - return 'no-referrer' + try { originAsURL = new URL(origin) } catch (e) { return false } + + if (originAsURL.protocol === 'https:' || originAsURL.protocol === 'wss:') { + return true + } + + if (/^127(?:\.[0-9]+){0,2}\.[0-9]+$|^(?:0*:)*?:?0*1$/.test(originAsURL.hostname) || + (originAsURL.hostname === 'localhost' || originAsURL.hostname.includes('localhost.')) || + (originAsURL.hostname.endsWith('.localhost'))) { + return true + } + + if (originAsURL.protocol === 'file:') return true + + return false + } } /** From dbec6e96ebcf5c46ce492d749016cb95a27c9e8a Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Tue, 1 Mar 2022 22:16:47 +0100 Subject: [PATCH 08/21] docs: add documentation about pottentially trustworthy --- lib/fetch/util.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index c841b3c08d0..b5537d74196 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -450,33 +450,41 @@ function isURLPotentiallyTrustworthy (url) { return false } + // If child of about, return true if (url.href === 'about:blank' || url.href === 'about:srcdoc') { return true } + // If scheme is data, return true if (url.protocol === 'data:') return true return isOriginPotentiallyTrustworthy(url.origin) - function isOriginPotentiallyTrustworthy(origin) { + function isOriginPotentiallyTrustworthy (origin) { + // If origin is explicitly null, return false if (origin == null || origin === 'null') return false let originAsURL + // If not valid because not semantically correct, return false try { originAsURL = new URL(origin) } catch (e) { return false } + // If secure, return true if (originAsURL.protocol === 'https:' || originAsURL.protocol === 'wss:') { return true } + // If localhost or variants, return true if (/^127(?:\.[0-9]+){0,2}\.[0-9]+$|^(?:0*:)*?:?0*1$/.test(originAsURL.hostname) || (originAsURL.hostname === 'localhost' || originAsURL.hostname.includes('localhost.')) || (originAsURL.hostname.endsWith('.localhost'))) { return true } + // If file, return true if (originAsURL.protocol === 'file:') return true + // If any other, return false return false } } From 6cfb076906ba2a5dc51653fddd2754427005b286 Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Tue, 1 Mar 2022 22:17:16 +0100 Subject: [PATCH 09/21] feat: expose pottentially trustworthy --- lib/fetch/util.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index b5537d74196..649eb4e8a8c 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -770,6 +770,8 @@ module.exports = { responseURL, responseLocationURL, isBlobLike, + isFileLike, + isURLPotentiallyTrustworthy, isValidReasonPhrase, sameOrigin, normalizeMethod, From 418f1192d7440b46922f11d67a952c1298549870 Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Tue, 1 Mar 2022 23:06:35 +0100 Subject: [PATCH 10/21] test: URL potentially trustworthy --- lib/fetch/util.js | 8 +++--- test/fetch/util.js | 65 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 4 deletions(-) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index 649eb4e8a8c..9bdd760a6bb 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -457,6 +457,9 @@ function isURLPotentiallyTrustworthy (url) { // If scheme is data, return true if (url.protocol === 'data:') return true + + // If file, return true + if (url.protocol === 'file:') return true return isOriginPotentiallyTrustworthy(url.origin) @@ -475,15 +478,12 @@ function isURLPotentiallyTrustworthy (url) { } // If localhost or variants, return true - if (/^127(?:\.[0-9]+){0,2}\.[0-9]+$|^(?:0*:)*?:?0*1$/.test(originAsURL.hostname) || + if (/^127(?:\.[0-9]+){0,2}\.[0-9]+$|^\[(?:0*:)*?:?0*1\]$/.test(originAsURL.hostname) || (originAsURL.hostname === 'localhost' || originAsURL.hostname.includes('localhost.')) || (originAsURL.hostname.endsWith('.localhost'))) { return true } - // If file, return true - if (originAsURL.protocol === 'file:') return true - // If any other, return false return false } diff --git a/test/fetch/util.js b/test/fetch/util.js index f1f55b4c2c2..97946695e56 100644 --- a/test/fetch/util.js +++ b/test/fetch/util.js @@ -113,3 +113,68 @@ test('sameOrigin', (t) => { t.end() }) + +test('CORBCheck', (t) => { + const allowedRequests = [{ + initiator: 'download', + currentURL: { scheme: '' } + }, { + initiator: '', + currentURL: { scheme: 'https' } + } + ] + + const response = { headersList: { get () { return '' } } } + + allowedRequests.forEach((request) => { + t.ok(util.CORBCheck(request, response)) + }) + + t.ok(util.CORBCheck({ + initiator: '', + currentURL: { scheme: '' } + }, response)) + + const protectedResponses = [{ + status: 206, + headersList: { get () { return 'text/html' } } + }, { + status: 206, + headersList: { get () { return 'application/javascript' } } + }, { + status: 206, + headersList: { get () { return 'application/xml' } } + }, { + status: 218, + headersList: { get (type) { return type === 'content-type' ? 'text/html' : 'x-content-type-options' } } + }] + + protectedResponses.forEach(response => { + t.equal(util.CORBCheck({ + initiator: '', + currentURL: { scheme: 'https' } + }, response), 'blocked') + }) + + t.end() +}) + +test('isURLPotentiallyTrustworthy', (t) => { + const valid = ['http://127.0.0.1', 'http://localhost.localhost', + 'http://[::1]', 'http://adb.localhost', 'https://something.com', 'wss://hello.com', + 'file:///link/to/file.txt', 'data:text/plain;base64,randomstring', 'about:blank', 'about:srcdoc'] + const invalid = ['http://121.3.4.5:55', 'null:8080', 'something:8080'] + + t.plan(valid.length + invalid.length + 1) + t.notOk(util.isURLPotentiallyTrustworthy('string')) + + for (const url of valid) { + const instance = new URL(url) + t.ok(util.isURLPotentiallyTrustworthy(instance)) + } + + for (const url of invalid) { + const instance = new URL(url) + t.notOk(util.isURLPotentiallyTrustworthy(instance)) + } +}) From ff4e6ad9f7f7b905604f63045f92985a0fce7990 Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Tue, 8 Mar 2022 13:00:19 +0100 Subject: [PATCH 11/21] fix: check for possibly undefined --- lib/fetch/util.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index 9bdd760a6bb..cadd251bb09 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -347,7 +347,7 @@ function determineRequestsReferrer (request) { // 3(a) If policy is "origin", then // 3(a)(I) If environment's global object is Window then if (request.referrer === 'client') { - if (environment.globalObject instanceof Window) { + if (environment?.globalObject instanceof Window) { const origin = environment.globalObject.self?.origin ?? environment.globalObject.location?.origin if (origin == null || origin === 'null') return 'no-referrer' @@ -355,7 +355,7 @@ function determineRequestsReferrer (request) { } else { // 3(a)(II) If environment's global object is not Window, // Let referrerSource be environments creationURL - if (environment.globalObject.location == null) { + if (environment?.globalObject?.location == null) { return 'no-referrer' } @@ -457,7 +457,7 @@ function isURLPotentiallyTrustworthy (url) { // If scheme is data, return true if (url.protocol === 'data:') return true - + // If file, return true if (url.protocol === 'file:') return true From bd547224943ad7bfb0d391fc23cfc1dfdaa4f2ef Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Tue, 8 Mar 2022 13:29:57 +0100 Subject: [PATCH 12/21] test: initial round --- test/fetch/util.js | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test/fetch/util.js b/test/fetch/util.js index 97946695e56..49f9588410b 100644 --- a/test/fetch/util.js +++ b/test/fetch/util.js @@ -178,3 +178,40 @@ test('isURLPotentiallyTrustworthy', (t) => { t.notOk(util.isURLPotentiallyTrustworthy(instance)) } }) + +test('determineRequestsReferrer', (t) => { + t.plan(4) + t.test('Should handle empty referrerPolicy', (tt) => { + tt.plan(2) + tt.equal(util.determineRequestsReferrer({}), 'no-referrer') + tt.equal(util.determineRequestsReferrer({ referrerPolicy: '' }), 'no-referrer') + }) + + t.test('Should handle "no-referrer" referrerPolicy', (tt) => { + tt.plan(1) + tt.equal(util.determineRequestsReferrer({ referrerPolicy: 'no-referrer' }), 'no-referrer') + }) + + t.test('Should return "no-referrer" if request referrer is absent', (tt) => { + tt.plan(1) + tt.equal(util.determineRequestsReferrer({ + referrerPolicy: 'origin' + }), 'no-referrer') + }) + + t.test('Should return "no-referrer" if scheme is local scheme', (tt) => { + tt.plan(4) + const referrerSources = [ + new URL('data:something'), + new URL('about:blank'), + new URL('javascript:something'), + new URL('file://path/to/file')] + + for (const source of referrerSources) { + tt.equal(util.determineRequestsReferrer({ + referrerPolicy: 'origin', + referrer: source + }), 'no-referrer') + } + }) +}) From c2bd3eb35f18a6ed4e011cbf96d8d111a80c7d9e Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Wed, 23 Mar 2022 23:06:38 +0100 Subject: [PATCH 13/21] feat: smaller improvements --- lib/fetch/util.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index cadd251bb09..dea69a1bfcd 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -337,7 +337,9 @@ function determineRequestsReferrer (request) { const policy = request.referrerPolicy // Return no-referrer when empty or policy says so - if (policy === '' || policy === 'no-referrer') return 'no-referrer' + if (policy == null || policy === '' || policy === 'no-referrer') { + return 'no-referrer' + } // 2. Let environment be the request client const environment = request.client @@ -374,7 +376,10 @@ function determineRequestsReferrer (request) { // If url's scheme is a local scheme (i.e. one of "about", "data", "javascript", "file") // then return "no-referrer". - if (urlProtocol === 'about:' || urlProtocol === 'data:' || urlProtocol === 'data:') { + if ( + urlProtocol === 'about:' || urlProtocol === 'data:' || + urlProtocol === 'javascript:' || urlProtocol === 'file:' + ) { return 'no-referrer' } From 5573e03cbba54d165a36461827d618f7432ebcbd Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Wed, 23 Mar 2022 23:22:39 +0100 Subject: [PATCH 14/21] docs: update in-code docs --- lib/fetch/util.js | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index dea69a1bfcd..0e881f57b9f 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -332,7 +332,6 @@ function clonePolicyContainer () { // https://w3c.github.io/webappsec-referrer-policy/#determine-requests-referrer function determineRequestsReferrer (request) { - // TODO // 1. Let policy be request's referrer policy. const policy = request.referrerPolicy @@ -345,14 +344,29 @@ function determineRequestsReferrer (request) { const environment = request.client let referrerSource = null - // TODO - // 3(a) If policy is "origin", then - // 3(a)(I) If environment's global object is Window then + /** + * 3, Switch on request’s referrer: + "client" + If environment’s global object is a Window object, then + Let document be the associated Document of environment’s global object. + If document’s origin is an opaque origin, return no referrer. + While document is an iframe srcdoc document, + let document be document’s browsing context’s browsing context container’s node document. + Let referrerSource be document’s URL. + + Otherwise, let referrerSource be environment’s creation URL. + + a URL + Let referrerSource be request’s referrer. + */ if (request.referrer === 'client') { if (environment?.globalObject instanceof Window) { const origin = environment.globalObject.self?.origin ?? environment.globalObject.location?.origin + // If document’s origin is an opaque origin, return no referrer. if (origin == null || origin === 'null') return 'no-referrer' + + // Let referrerSource be document’s URL. referrerSource = new URL(environment.globalObject.location.href) } else { // 3(a)(II) If environment's global object is not Window, From aaddf0da6f23e3ac4c2cd3c5e1338ac57c59dd82 Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Wed, 23 Mar 2022 23:25:43 +0100 Subject: [PATCH 15/21] lint: ignore line --- lib/fetch/util.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index 0e881f57b9f..dd9367209b4 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -360,7 +360,8 @@ function determineRequestsReferrer (request) { Let referrerSource be request’s referrer. */ if (request.referrer === 'client') { - if (environment?.globalObject instanceof Window) { + // Not defined in Node but part of the spec + if (environment?.globalObject instanceof Window) { // eslint-disable-line const origin = environment.globalObject.self?.origin ?? environment.globalObject.location?.origin // If document’s origin is an opaque origin, return no referrer. From 7bd6182b7cb83727c38736d3005501fb157afb2e Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Thu, 24 Mar 2022 00:12:00 +0100 Subject: [PATCH 16/21] tests: add more test scenarios --- test/fetch/util.js | 42 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/test/fetch/util.js b/test/fetch/util.js index 49f9588410b..d91dba3e9e6 100644 --- a/test/fetch/util.js +++ b/test/fetch/util.js @@ -180,7 +180,8 @@ test('isURLPotentiallyTrustworthy', (t) => { }) test('determineRequestsReferrer', (t) => { - t.plan(4) + t.plan(7) + t.test('Should handle empty referrerPolicy', (tt) => { tt.plan(2) tt.equal(util.determineRequestsReferrer({}), 'no-referrer') @@ -214,4 +215,43 @@ test('determineRequestsReferrer', (t) => { }), 'no-referrer') } }) + + t.test('Should return "no-referrer" if the request referrer is neither client nor instance of URL', (tt) => { + tt.plan(4) + const requests = [ + { referrerPolicy: 'origin', referrer: 'string' }, + { referrerPolicy: 'origin', referrer: null }, + { referrerPolicy: 'origin', referrer: undefined }, + { referrerPolicy: 'origin', referrer: '' } + ] + + for (const request of requests) { + tt.equal(util.determineRequestsReferrer(request), 'no-referrer') + } + }) + + t.test('Should return referrer origin on referrerPolicy origin', (tt) => { + tt.plan(1) + const expectedRequest = { + referrerPolicy: 'origin', + referrer: new URL('http://example:12345@example.com') + } + + tt.equal(util.determineRequestsReferrer(expectedRequest), expectedRequest.referrer.origin) + }) + + t.test('Should return referrer url on referrerPolicy unsafe-url', (tt) => { + tt.plan(1) + const expectedRequest = { + referrerPolicy: 'unsafe-url', + referrer: new URL('http://example:12345@example.com/hello/world') + } + + const expectedReffererUrl = new URL(expectedRequest.referrer.href) + + expectedReffererUrl.username = '' + expectedReffererUrl.password = '' + + tt.equal(util.determineRequestsReferrer(expectedRequest), expectedReffererUrl.href) + }) }) From 215dc9aa49237faa4d8a36095baf313df2834545 Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Thu, 24 Mar 2022 00:12:26 +0100 Subject: [PATCH 17/21] refactor: small improvements --- lib/fetch/util.js | 53 ++++++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index dd9367209b4..4b8294c1ed9 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -383,7 +383,8 @@ function determineRequestsReferrer (request) { // referrerSource be requests's referrer. referrerSource = request.referrer } else { - // If referrerSource is null, then return "no-referrer". + // If referrerSource neither client nor instance of URL + // then return "no-referrer". return 'no-referrer' } @@ -407,7 +408,7 @@ function determineRequestsReferrer (request) { // source for use as referrer (using util function, with originOnly true) // 6. If result of seralizing referrerUrl is a string whose length is greater than // 4096, then set referrerURL to referrerOrigin - ? referrerOrigin = stripURLForReferrer(referrerSource, true) + ? (referrerOrigin = stripURLForReferrer(referrerSource, true)) : temp const areSameOrigin = sameOrigin(request, referrerUrl) const isNonPotentiallyTrustWorthy = isURLPotentiallyTrustworthy(referrerUrl) && @@ -416,37 +417,38 @@ function determineRequestsReferrer (request) { // NOTE: How to treat step 7? // 8. Execute the switch statements corresponding to the value of policy: switch (policy) { - case 'origin': return referrerOrigin + case 'origin': return referrerOrigin ?? stripURLForReferrer(referrerSource, true) case 'unsafe-url': return referrerUrl - case 'strict-origin': - /** - * 1. If referrerURL is a potentially trustworthy URL and - * request’s current URL is not a potentially trustworthy URL, - * then return no referrer. - * 2. Return referrerOrigin - */ - return isNonPotentiallyTrustWorthy ? 'no-referrer' : referrerOrigin - case 'strict-origin-when-cross-origin': - /** - * 1. If the origin of referrerURL and the origin of request’s current URL are the same, - * then return referrerURL. - * 2. If referrerURL is a potentially trustworthy URL and request’s current URL is not a - * potentially trustworthy URL, then return no referrer. - * 3. Return referrerOrigin - */ - if (areSameOrigin) return referrerOrigin - else return isNonPotentiallyTrustWorthy ? 'no-referrer' : referrerOrigin case 'same-origin': return areSameOrigin ? referrerOrigin : 'no-referrer' case 'origin-when-cross-origin': return areSameOrigin ? referrerUrl : referrerOrigin - case 'no-referrer-when-downgrade': + case 'strict-origin-when-cross-origin': + /** + * 1. If the origin of referrerURL and the origin of request’s current URL are the same, + * then return referrerURL. + * 2. If referrerURL is a potentially trustworthy URL and request’s current URL is not a + * potentially trustworthy URL, then return no referrer. + * 3. Return referrerOrigin + */ + if (areSameOrigin) return referrerOrigin + // else return isNonPotentiallyTrustWorthy ? 'no-referrer' : referrerOrigin + case 'strict-origin': // eslint-disable-line + /** + * 1. If referrerURL is a potentially trustworthy URL and + * request’s current URL is not a potentially trustworthy URL, + * then return no referrer. + * 2. Return referrerOrigin + */ + case 'no-referrer-when-downgrade': // eslint-disable-line /** * 1. If referrerURL is a potentially trustworthy URL and * request’s current URL is not a potentially trustworthy URL, * then return no referrer. * 2. Return referrerOrigin */ + + default: // eslint-disable-line return isNonPotentiallyTrustWorthy ? 'no-referrer' : referrerOrigin } @@ -456,12 +458,7 @@ function determineRequestsReferrer (request) { urlObject.password = '' urlObject.hash = '' - if (originOnly) { - urlObject.pathname = '' - urlObject.search = '' - } - - return urlObject.href + return originOnly ? urlObject.origin : urlObject.href } } From 6ed09c17be399e459f9174dbaf4fd30206bcb8c5 Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Fri, 23 Sep 2022 09:04:17 +0200 Subject: [PATCH 18/21] refactor: apply review --- lib/fetch/util.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index 4b8294c1ed9..fe77699302f 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -394,7 +394,7 @@ function determineRequestsReferrer (request) { // then return "no-referrer". if ( urlProtocol === 'about:' || urlProtocol === 'data:' || - urlProtocol === 'javascript:' || urlProtocol === 'file:' + urlProtocol === 'blob:' ) { return 'no-referrer' } @@ -787,7 +787,6 @@ module.exports = { responseURL, responseLocationURL, isBlobLike, - isFileLike, isURLPotentiallyTrustworthy, isValidReasonPhrase, sameOrigin, From e5ff949ec8eb6fede8216f2d151ae38209d77348 Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Fri, 23 Sep 2022 09:04:40 +0200 Subject: [PATCH 19/21] tests: adjust testing --- test/fetch/util.js | 50 ++-------------------------------------------- 1 file changed, 2 insertions(+), 48 deletions(-) diff --git a/test/fetch/util.js b/test/fetch/util.js index d91dba3e9e6..fcd9d94cbbe 100644 --- a/test/fetch/util.js +++ b/test/fetch/util.js @@ -114,51 +114,6 @@ test('sameOrigin', (t) => { t.end() }) -test('CORBCheck', (t) => { - const allowedRequests = [{ - initiator: 'download', - currentURL: { scheme: '' } - }, { - initiator: '', - currentURL: { scheme: 'https' } - } - ] - - const response = { headersList: { get () { return '' } } } - - allowedRequests.forEach((request) => { - t.ok(util.CORBCheck(request, response)) - }) - - t.ok(util.CORBCheck({ - initiator: '', - currentURL: { scheme: '' } - }, response)) - - const protectedResponses = [{ - status: 206, - headersList: { get () { return 'text/html' } } - }, { - status: 206, - headersList: { get () { return 'application/javascript' } } - }, { - status: 206, - headersList: { get () { return 'application/xml' } } - }, { - status: 218, - headersList: { get (type) { return type === 'content-type' ? 'text/html' : 'x-content-type-options' } } - }] - - protectedResponses.forEach(response => { - t.equal(util.CORBCheck({ - initiator: '', - currentURL: { scheme: 'https' } - }, response), 'blocked') - }) - - t.end() -}) - test('isURLPotentiallyTrustworthy', (t) => { const valid = ['http://127.0.0.1', 'http://localhost.localhost', 'http://[::1]', 'http://adb.localhost', 'https://something.com', 'wss://hello.com', @@ -201,12 +156,11 @@ test('determineRequestsReferrer', (t) => { }) t.test('Should return "no-referrer" if scheme is local scheme', (tt) => { - tt.plan(4) + tt.plan(3) const referrerSources = [ new URL('data:something'), new URL('about:blank'), - new URL('javascript:something'), - new URL('file://path/to/file')] + new URL('blob:https://video_url')] for (const source of referrerSources) { tt.equal(util.determineRequestsReferrer({ From 681a75525517a9a2c6e2a2bf33e41327603bb7b7 Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Mon, 26 Sep 2022 08:48:25 +0200 Subject: [PATCH 20/21] refactor: apply PR review --- lib/fetch/util.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index fe77699302f..dcfc0f542d6 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -361,7 +361,7 @@ function determineRequestsReferrer (request) { */ if (request.referrer === 'client') { // Not defined in Node but part of the spec - if (environment?.globalObject instanceof Window) { // eslint-disable-line + if (request.client?.globalObject?.constructor?.name === 'Window' ) { // eslint-disable-line const origin = environment.globalObject.self?.origin ?? environment.globalObject.location?.origin // If document’s origin is an opaque origin, return no referrer. From 2ca09418fe8414e330ee12b7807f289735215f6b Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Mon, 26 Sep 2022 09:53:02 +0200 Subject: [PATCH 21/21] refactor: smaller adjustements --- lib/fetch/util.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/fetch/util.js b/lib/fetch/util.js index dcfc0f542d6..6cf628ccfb9 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -406,9 +406,9 @@ function determineRequestsReferrer (request) { const referrerUrl = (temp = stripURLForReferrer(referrerSource)).length > 4096 // 5. Let referrerOrigin be the result of stripping referrer // source for use as referrer (using util function, with originOnly true) + ? (referrerOrigin = stripURLForReferrer(referrerSource, true)) // 6. If result of seralizing referrerUrl is a string whose length is greater than // 4096, then set referrerURL to referrerOrigin - ? (referrerOrigin = stripURLForReferrer(referrerSource, true)) : temp const areSameOrigin = sameOrigin(request, referrerUrl) const isNonPotentiallyTrustWorthy = isURLPotentiallyTrustworthy(referrerUrl) && @@ -417,7 +417,7 @@ function determineRequestsReferrer (request) { // NOTE: How to treat step 7? // 8. Execute the switch statements corresponding to the value of policy: switch (policy) { - case 'origin': return referrerOrigin ?? stripURLForReferrer(referrerSource, true) + case 'origin': return referrerOrigin != null ? referrerOrigin : stripURLForReferrer(referrerSource, true) case 'unsafe-url': return referrerUrl case 'same-origin': return areSameOrigin ? referrerOrigin : 'no-referrer' @@ -484,10 +484,7 @@ function isURLPotentiallyTrustworthy (url) { // If origin is explicitly null, return false if (origin == null || origin === 'null') return false - let originAsURL - - // If not valid because not semantically correct, return false - try { originAsURL = new URL(origin) } catch (e) { return false } + const originAsURL = new URL(origin) // If secure, return true if (originAsURL.protocol === 'https:' || originAsURL.protocol === 'wss:') {