hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-25 16:55:19 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #13771 from github/max-schaefer/server-side-url-redirect-help

Browse Source 
JavaScript: Improve query help for `js/server-side-unvalidated-url-redirection`.
This commit is contained in:
Max Schaefer
2023-09-13 13:20:48 +01:00
committed by GitHub
parent 62b41799d2 a9e81672f0
commit e722e3288f
8 changed files with 95 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
javascript/ql/src/Security/CWE-601/ServerSideUrlRedirect.qhelp
View File

@@ -16,6 +16,10 @@ To guard against untrusted URL redirection, it is advisable to avoid putting use
directly into a redirect URL. Instead, maintain a list of authorized
redirects on the server; then choose from that list based on the user input provided.
</p>
<p>
If this is not possible, then the user input should be validated in some other way,
for example, by verifying that the target URL is on the same host as the current page.
</p>
</recommendation>
<example>
@@ -32,6 +36,21 @@ before doing the redirection:
</p>
<sample src="examples/ServerSideUrlRedirectGood.js"/>
<p>
Alternatively, we can check that the target URL does not redirect to a different host
by parsing it relative to a base URL with a known host and verifying that the host
stays the same:
</p>
<sample src="examples/ServerSideUrlRedirectGood2.js"/>
<p>
Note that as written, the above code will allow redirects to URLs on <code>example.com</code>,
which is harmless but perhaps not intended. You can substitute your own domain (if known) for
<code>example.com</code> to prevent this.
</p>
</example>
<references>

4
javascript/ql/src/Security/CWE-601/examples/ServerSideUrlRedirect.js
View File

@@ -1,6 +1,6 @@
const app = require("express")();
app.get('/some/path', function(req, res) {
app.get("/redirect", function (req, res) {
// BAD: a request parameter is incorporated without validation into a URL redirect
res.redirect(req.param("target"));
res.redirect(req.query["target"]);
});

9
javascript/ql/src/Security/CWE-601/examples/ServerSideUrlRedirectGood.js
View File

@@ -2,9 +2,12 @@ const app = require("express")();
const VALID_REDIRECT = "http://cwe.mitre.org/data/definitions/601.html";
app.get('/some/path', function(req, res) {
app.get("/redirect", function (req, res) {
// GOOD: the request parameter is validated against a known fixed string
let target = req.param("target");
if (VALID_REDIRECT === target)
let target = req.query["target"];
if (VALID_REDIRECT === target) {
res.redirect(target);
} else {
res.redirect("/");
}
});

22
javascript/ql/src/Security/CWE-601/examples/ServerSideUrlRedirectGood2.js Normal file
View File

@@ -0,0 +1,22 @@
const app = require("express")();
function isLocalUrl(path) {
try {
return (
// TODO: consider substituting your own domain for example.com
new URL(path, "https://example.com").origin === "https://example.com"
);
} catch (e) {
return false;
}
}
app.get("/redirect", function (req, res) {
// GOOD: check that we don't redirect to a different host
let target = req.query["target"];
if (isLocalUrl(target)) {
res.redirect(target);
} else {
res.redirect("/");
}
});

5
javascript/ql/test/query-tests/Security/CWE-601/ServerSideUrlRedirect/ServerSideUrlRedirect.expected
View File

@@ -1,4 +1,7 @@
nodes
| ServerSideUrlRedirect.js:5:16:5:34 | req.query["target"] |
| ServerSideUrlRedirect.js:5:16:5:34 | req.query["target"] |
| ServerSideUrlRedirect.js:5:16:5:34 | req.query["target"] |
| express.js:7:16:7:34 | req.param("target") |
| express.js:7:16:7:34 | req.param("target") |
| express.js:7:16:7:34 | req.param("target") |
@@ -114,6 +117,7 @@ nodes
| react-native.js:9:26:9:32 | tainted |
| react-native.js:9:26:9:32 | tainted |
edges
| ServerSideUrlRedirect.js:5:16:5:34 | req.query["target"] | ServerSideUrlRedirect.js:5:16:5:34 | req.query["target"] |
| express.js:7:16:7:34 | req.param("target") | express.js:7:16:7:34 | req.param("target") |
| express.js:12:26:12:44 | req.param("target") | express.js:12:26:12:44 | req.param("target") |
| express.js:27:7:27:34 | target | express.js:33:18:33:23 | target |
@@ -211,6 +215,7 @@ edges
| react-native.js:7:17:7:33 | req.param("code") | react-native.js:7:7:7:33 | tainted |
| react-native.js:7:17:7:33 | req.param("code") | react-native.js:7:7:7:33 | tainted |
#select
| ServerSideUrlRedirect.js:5:16:5:34 | req.query["target"] | ServerSideUrlRedirect.js:5:16:5:34 | req.query["target"] | ServerSideUrlRedirect.js:5:16:5:34 | req.query["target"] | Untrusted URL redirection depends on a $@. | ServerSideUrlRedirect.js:5:16:5:34 | req.query["target"] | user-provided value |
| express.js:7:16:7:34 | req.param("target") | express.js:7:16:7:34 | req.param("target") | express.js:7:16:7:34 | req.param("target") | Untrusted URL redirection depends on a $@. | express.js:7:16:7:34 | req.param("target") | user-provided value |
| express.js:12:26:12:44 | req.param("target") | express.js:12:26:12:44 | req.param("target") | express.js:12:26:12:44 | req.param("target") | Untrusted URL redirection depends on a $@. | express.js:12:26:12:44 | req.param("target") | user-provided value |
| express.js:33:18:33:23 | target | express.js:27:16:27:34 | req.param("target") | express.js:33:18:33:23 | target | Untrusted URL redirection depends on a $@. | express.js:27:16:27:34 | req.param("target") | user-provided value |

6
javascript/ql/test/query-tests/Security/CWE-601/ServerSideUrlRedirect/ServerSideUrlRedirect.js Normal file
View File

@@ -0,0 +1,6 @@
const app = require("express")();
app.get("/redirect", function (req, res) {
// BAD: a request parameter is incorporated without validation into a URL redirect
res.redirect(req.query["target"]);
});

13
javascript/ql/test/query-tests/Security/CWE-601/ServerSideUrlRedirect/ServerSideUrlRedirectGood.js Normal file
View File

@@ -0,0 +1,13 @@
const app = require("express")();
const VALID_REDIRECT = "http://cwe.mitre.org/data/definitions/601.html";
app.get("/redirect", function (req, res) {
// GOOD: the request parameter is validated against a known fixed string
let target = req.query["target"];
if (VALID_REDIRECT === target) {
res.redirect(target);
} else {
res.redirect("/");
}
});

22
javascript/ql/test/query-tests/Security/CWE-601/ServerSideUrlRedirect/ServerSideUrlRedirectGood2.js Normal file
View File

@@ -0,0 +1,22 @@
const app = require("express")();
function isLocalUrl(path) {
try {
return (
// TODO: consider substituting your own domain for example.com
new URL(path, "https://example.com").origin === "https://example.com"
);
} catch (e) {
return false;
}
}
app.get("/redirect", function (req, res) {
// GOOD: check that we don't redirect to a different host
let target = req.query["target"];
if (isLocalUrl(target)) {
res.redirect(target);
} else {
res.redirect("/");
}
});