hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-23 07:45:17 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #15893 from erik-krogh/more-filter-taint

Browse Source 
JS: allow more flow through .filter()
This commit is contained in:
Erik Krogh Kristensen
2024-03-13 16:19:28 +01:00
committed by GitHub
parent 53502a8662 129286aa1c
commit bd121b98ae
4 changed files with 61 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
javascript/ql/lib/semmle/javascript/Arrays.qll
View File

@@ -36,7 +36,8 @@ module ArrayTaintTracking {
succ = call
)
or
// `array.filter(x => x)` and `array.filter(x => !!x)` keeps the taint
// `array.filter(x => x)` and `array.filter(x => !<something>)` keeps the taint
// the latter is assumed to filter away only specific values, thus keeping the taint
call.(DataFlow::MethodCallNode).getMethodName() = "filter" and
pred = call.getReceiver() and
succ = call and
@@ -47,7 +48,7 @@ module ArrayTaintTracking {
|
param = ret
or
param = DataFlow::exprNode(ret.asExpr().(LogNotExpr).getOperand().(LogNotExpr).getOperand())
ret.asExpr() instanceof LogNotExpr
)
or
// `array.reduce` with tainted value in callback

42
javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected
View File

@@ -84,6 +84,26 @@ nodes
| ReflectedXss.js:110:16:110:30 | request.query.p |
| ReflectedXss.js:110:16:110:30 | request.query.p |
| ReflectedXss.js:110:16:110:30 | request.query.p |
| ReflectedXss.js:114:11:114:41 | queryKeys |
| ReflectedXss.js:114:13:114:27 | keys: queryKeys |
| ReflectedXss.js:114:13:114:27 | keys: queryKeys |
| ReflectedXss.js:116:11:116:45 | keys |
| ReflectedXss.js:116:18:116:26 | queryKeys |
| ReflectedXss.js:116:18:116:45 | queryKe ... s?.keys |
| ReflectedXss.js:116:31:116:45 | paramKeys?.keys |
| ReflectedXss.js:116:31:116:45 | paramKeys?.keys |
| ReflectedXss.js:118:11:118:61 | keyArray |
| ReflectedXss.js:118:22:118:61 | typeof ... : keys |
| ReflectedXss.js:118:49:118:54 | [keys] |
| ReflectedXss.js:118:50:118:53 | keys |
| ReflectedXss.js:118:58:118:61 | keys |
| ReflectedXss.js:119:11:119:72 | invalidKeys |
| ReflectedXss.js:119:25:119:32 | keyArray |
| ReflectedXss.js:119:25:119:72 | keyArra ... s(key)) |
| ReflectedXss.js:122:30:122:73 | `${inva ... telist` |
| ReflectedXss.js:122:30:122:73 | `${inva ... telist` |
| ReflectedXss.js:122:33:122:43 | invalidKeys |
| ReflectedXss.js:122:33:122:54 | invalid ... n(', ') |
| ReflectedXssContentTypes.js:10:14:10:36 | "FOO: " ... rams.id |
| ReflectedXssContentTypes.js:10:14:10:36 | "FOO: " ... rams.id |
| ReflectedXssContentTypes.js:10:24:10:36 | req.params.id |
@@ -307,6 +327,26 @@ edges
| ReflectedXss.js:103:76:103:83 | req.body | ReflectedXss.js:103:12:103:84 | markdow ... q.body) |
| ReflectedXss.js:103:76:103:83 | req.body | ReflectedXss.js:103:12:103:84 | markdow ... q.body) |
| ReflectedXss.js:110:16:110:30 | request.query.p | ReflectedXss.js:110:16:110:30 | request.query.p |
| ReflectedXss.js:114:11:114:41 | queryKeys | ReflectedXss.js:116:18:116:26 | queryKeys |
| ReflectedXss.js:114:13:114:27 | keys: queryKeys | ReflectedXss.js:114:11:114:41 | queryKeys |
| ReflectedXss.js:114:13:114:27 | keys: queryKeys | ReflectedXss.js:114:11:114:41 | queryKeys |
| ReflectedXss.js:116:11:116:45 | keys | ReflectedXss.js:118:50:118:53 | keys |
| ReflectedXss.js:116:11:116:45 | keys | ReflectedXss.js:118:58:118:61 | keys |
| ReflectedXss.js:116:18:116:26 | queryKeys | ReflectedXss.js:116:18:116:45 | queryKe ... s?.keys |
| ReflectedXss.js:116:18:116:45 | queryKe ... s?.keys | ReflectedXss.js:116:11:116:45 | keys |
| ReflectedXss.js:116:31:116:45 | paramKeys?.keys | ReflectedXss.js:116:18:116:45 | queryKe ... s?.keys |
| ReflectedXss.js:116:31:116:45 | paramKeys?.keys | ReflectedXss.js:116:18:116:45 | queryKe ... s?.keys |
| ReflectedXss.js:118:11:118:61 | keyArray | ReflectedXss.js:119:25:119:32 | keyArray |
| ReflectedXss.js:118:22:118:61 | typeof ... : keys | ReflectedXss.js:118:11:118:61 | keyArray |
| ReflectedXss.js:118:49:118:54 | [keys] | ReflectedXss.js:118:22:118:61 | typeof ... : keys |
| ReflectedXss.js:118:50:118:53 | keys | ReflectedXss.js:118:49:118:54 | [keys] |
| ReflectedXss.js:118:58:118:61 | keys | ReflectedXss.js:118:22:118:61 | typeof ... : keys |
| ReflectedXss.js:119:11:119:72 | invalidKeys | ReflectedXss.js:122:33:122:43 | invalidKeys |
| ReflectedXss.js:119:25:119:32 | keyArray | ReflectedXss.js:119:25:119:72 | keyArra ... s(key)) |
| ReflectedXss.js:119:25:119:72 | keyArra ... s(key)) | ReflectedXss.js:119:11:119:72 | invalidKeys |
| ReflectedXss.js:122:33:122:43 | invalidKeys | ReflectedXss.js:122:33:122:54 | invalid ... n(', ') |
| ReflectedXss.js:122:33:122:54 | invalid ... n(', ') | ReflectedXss.js:122:30:122:73 | `${inva ... telist` |
| ReflectedXss.js:122:33:122:54 | invalid ... n(', ') | ReflectedXss.js:122:30:122:73 | `${inva ... telist` |
| ReflectedXssContentTypes.js:10:24:10:36 | req.params.id | ReflectedXssContentTypes.js:10:14:10:36 | "FOO: " ... rams.id |
| ReflectedXssContentTypes.js:10:24:10:36 | req.params.id | ReflectedXssContentTypes.js:10:14:10:36 | "FOO: " ... rams.id |
| ReflectedXssContentTypes.js:10:24:10:36 | req.params.id | ReflectedXssContentTypes.js:10:14:10:36 | "FOO: " ... rams.id |
@@ -461,6 +501,8 @@ edges
| ReflectedXss.js:100:12:100:39 | markdow ... q.body) | ReflectedXss.js:100:31:100:38 | req.body | ReflectedXss.js:100:12:100:39 | markdow ... q.body) | Cross-site scripting vulnerability due to a $@. | ReflectedXss.js:100:31:100:38 | req.body | user-provided value |
| ReflectedXss.js:103:12:103:84 | markdow ... q.body) | ReflectedXss.js:103:76:103:83 | req.body | ReflectedXss.js:103:12:103:84 | markdow ... q.body) | Cross-site scripting vulnerability due to a $@. | ReflectedXss.js:103:76:103:83 | req.body | user-provided value |
| ReflectedXss.js:110:16:110:30 | request.query.p | ReflectedXss.js:110:16:110:30 | request.query.p | ReflectedXss.js:110:16:110:30 | request.query.p | Cross-site scripting vulnerability due to a $@. | ReflectedXss.js:110:16:110:30 | request.query.p | user-provided value |
| ReflectedXss.js:122:30:122:73 | `${inva ... telist` | ReflectedXss.js:114:13:114:27 | keys: queryKeys | ReflectedXss.js:122:30:122:73 | `${inva ... telist` | Cross-site scripting vulnerability due to a $@. | ReflectedXss.js:114:13:114:27 | keys: queryKeys | user-provided value |
| ReflectedXss.js:122:30:122:73 | `${inva ... telist` | ReflectedXss.js:116:31:116:45 | paramKeys?.keys | ReflectedXss.js:122:30:122:73 | `${inva ... telist` | Cross-site scripting vulnerability due to a $@. | ReflectedXss.js:116:31:116:45 | paramKeys?.keys | user-provided value |
| ReflectedXssContentTypes.js:10:14:10:36 | "FOO: " ... rams.id | ReflectedXssContentTypes.js:10:24:10:36 | req.params.id | ReflectedXssContentTypes.js:10:14:10:36 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to a $@. | ReflectedXssContentTypes.js:10:24:10:36 | req.params.id | user-provided value |
| ReflectedXssContentTypes.js:20:14:20:36 | "FOO: " ... rams.id | ReflectedXssContentTypes.js:20:24:20:36 | req.params.id | ReflectedXssContentTypes.js:20:14:20:36 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to a $@. | ReflectedXssContentTypes.js:20:24:20:36 | req.params.id | user-provided value |
| ReflectedXssContentTypes.js:39:13:39:35 | "FOO: " ... rams.id | ReflectedXssContentTypes.js:39:23:39:35 | req.params.id | ReflectedXssContentTypes.js:39:13:39:35 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to a $@. | ReflectedXssContentTypes.js:39:23:39:35 | req.params.id | user-provided value |

14
javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.js
View File

@@ -109,3 +109,17 @@ hapi.route({
handler: function (request){
return request.query.p; // NOT OK
}});
app.get("invalid/keys/:id", async (req, res) => {
const { keys: queryKeys } = req.query;
const paramKeys = req.params;
const keys = queryKeys || paramKeys?.keys;
const keyArray = typeof keys === 'string' ? [keys] : keys;
const invalidKeys = keyArray.filter(key => !whitelist.includes(key));
if (invalidKeys.length) {
res.status(400).send(`${invalidKeys.join(', ')} not in whitelist`);
return;
}
});

2
javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected
View File

@@ -19,6 +19,8 @@
| ReflectedXss.js:100:12:100:39 | markdow ... q.body) | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:100:31:100:38 | req.body | user-provided value |
| ReflectedXss.js:103:12:103:84 | markdow ... q.body) | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:103:76:103:83 | req.body | user-provided value |
| ReflectedXss.js:110:16:110:30 | request.query.p | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:110:16:110:30 | request.query.p | user-provided value |
| ReflectedXss.js:122:30:122:73 | `${inva ... telist` | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:114:13:114:27 | keys: queryKeys | user-provided value |
| ReflectedXss.js:122:30:122:73 | `${inva ... telist` | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:116:31:116:45 | paramKeys?.keys | user-provided value |
| ReflectedXssContentTypes.js:10:14:10:36 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to $@. | ReflectedXssContentTypes.js:10:24:10:36 | req.params.id | user-provided value |
| ReflectedXssContentTypes.js:20:14:20:36 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to $@. | ReflectedXssContentTypes.js:20:24:20:36 | req.params.id | user-provided value |
| ReflectedXssContentTypes.js:39:13:39:35 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to $@. | ReflectedXssContentTypes.js:39:23:39:35 | req.params.id | user-provided value |