Skip to content

Commit f4cf7b6

Browse files
authored
Guard OAuth return pages against prototype pollution (#281)
* Guard query-param parsing against prototype pollution oauthReturn.html and mockOAuthAuthorization.html built a queryParams object by writing map[name] = value for every URL parameter, where the name is user-controlled. A parameter named __proto__ (or constructor / prototype) could therefore pollute Object.prototype (CodeQL js/remote-property-injection, CWE-1321). Skip those reserved names when populating the map. Legitimate parameters (state, code, redirect_uri) are unaffected. * Read OAuth query parameters by name (CodeQL js/remote-property-injection) The __proto__/constructor/prototype blocklist added earlier is not recognized as a sanitizer by CodeQL, and the alert covers any property write keyed by user input, not just prototype pollution. Replace the query-parameter map with a getQueryParam(name) helper so the pages only read the parameters they use (state, code, redirect_uri) and never write a property under a user-controlled name. * Restrict mock OAuth redirect_uri to the current origin (CodeQL js/xss, js/client-side-unvalidated-url-redirection) * Parse redirect_uri with the URL API instead of an anchor element CodeQL flags assigning user input to an anchor's href as an XSS and unvalidated-redirect sink (js/xss, js/client-side-unvalidated-url-redirection) even though the element is detached and only used to parse the URL. Use new URL() for parsing so no sink is involved, and reject a missing redirect_uri explicitly instead of letting it resolve to "/undefined". * Rebuild the mock redirect target from url.origin The origin equality check has already passed at this point, so url.origin + url.pathname is equivalent to the manual protocol/host concatenation and reads clearer. Also document that any query or fragment on the incoming redirect_uri is dropped deliberately - the caller appends its own ?code=...&state=... parameters.
1 parent 27487bc commit f4cf7b6

2 files changed

Lines changed: 63 additions & 13 deletions

File tree

ui/commons/src/main/resources/oauthReturn.html

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
<!-- Copyright 2015 ForgeRock AS.
2+
License terms: https://forgerock.org/cddlv1-0/
3+
Portions Copyrighted 2026 3A Systems, LLC -->
14
<script>
25
(function () {
36
/**
@@ -14,15 +17,22 @@
1417
This step is necessary because IDPs do not allow you to specify a redirect URI with
1518
a hash fragment; this file works around that requirement.
1619
*/
17-
var queryParams = window.location.search.replace("?", "").split("&").reduce(function (map, item) {
20+
// Look up a single query parameter by name. Reading only the known
21+
// parameters avoids writing map entries keyed by user input (CWE-1321).
22+
function getQueryParam(name) {
23+
var value;
24+
window.location.search.replace("?", "").split("&").forEach(function (item) {
1825
var parts = item.split("=");
19-
map[parts[0]] = decodeURIComponent(parts[1]);
20-
return map;
21-
}, {});
26+
if (parts[0] === name) {
27+
value = decodeURIComponent(parts[1]);
28+
}
29+
});
30+
return value;
31+
}
2232

2333
window.location.href = window.location.href
24-
// queryParams.state is expected to be the URL fragment plus any other
34+
// the "state" parameter is expected to be the URL fragment plus any other
2535
// details necessary to resume the oauth process
26-
.replace(/oauthReturn\.html.*/, "#" + queryParams.state + "&code=" + queryParams.code);
36+
.replace(/oauthReturn\.html.*/, "#" + getQueryParam("state") + "&code=" + getQueryParam("code"));
2737
}())
2838
</script>

ui/mock/src/main/resources/mockOAuthAuthorization.html

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11

22

3+
<!-- Copyright 2015 ForgeRock AS.
4+
License terms: https://forgerock.org/cddlv1-0/
5+
Portions Copyrighted 2026 3A Systems, LLC -->
36
<body>
47

58
Mock OAuth Provider redirecting in <span id="countDown"></span>....
@@ -9,18 +12,55 @@
912

1013
<script>
1114
(function () {
12-
var queryParams = window.location.search.replace("?", "").split("&").reduce(function (map, item) {
15+
// Look up a single query parameter by name. Reading only the known
16+
// parameters avoids writing map entries keyed by user input (CWE-1321).
17+
function getQueryParam(name) {
18+
var value;
19+
window.location.search.replace("?", "").split("&").forEach(function (item) {
1320
var parts = item.split("=");
14-
map[parts[0]] = decodeURIComponent(parts[1]);
15-
return map;
16-
}, {}),
17-
countDown = 3;
21+
if (parts[0] === name) {
22+
value = decodeURIComponent(parts[1]);
23+
}
24+
});
25+
return value;
26+
}
1827

19-
setInterval(function () {
28+
// The mock provider only ever redirects back into the application that
29+
// initiated the flow, so resolve redirect_uri against the current
30+
// document and rebuild it on our own origin. Cross-origin targets and
31+
// non-http schemes (e.g. javascript:) are rejected (CWE-79, CWE-601).
32+
function getSafeRedirectUri() {
33+
var redirectUri = getQueryParam("redirect_uri");
34+
if (!redirectUri) {
35+
return undefined;
36+
}
37+
var url;
38+
try {
39+
url = new URL(redirectUri, window.location.href);
40+
} catch (e) {
41+
return undefined;
42+
}
43+
if (url.origin !== window.location.origin) {
44+
return undefined;
45+
}
46+
// Any query or fragment on the incoming redirect_uri is dropped
47+
// deliberately: the caller appends its own ?code=...&state=... below.
48+
return url.origin + url.pathname;
49+
}
50+
51+
var countDown = 3;
52+
53+
var timer = setInterval(function () {
2054
document.getElementById("countDown").innerHTML = countDown;
2155
countDown--;
2256
if (countDown <= 0) {
23-
window.location.href=queryParams.redirect_uri + "?code=randomAccessCode&state=" + encodeURIComponent(queryParams.state);
57+
clearInterval(timer);
58+
var redirectUri = getSafeRedirectUri();
59+
if (redirectUri) {
60+
window.location.href=redirectUri + "?code=randomAccessCode&state=" + encodeURIComponent(getQueryParam("state"));
61+
} else {
62+
document.body.innerHTML = "Invalid redirect_uri";
63+
}
2464
}
2565
}, 1000);
2666
document.getElementById("countDown").innerHTML = countDown+1;

0 commit comments

Comments
 (0)