hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-05-01 03:35:13 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #7044 from asgerf/js/proto-pollution-fps

Browse Source 
Approved by erik-krogh
This commit is contained in:
CodeQL CI
2021-11-04 02:45:46 -07:00
committed by GitHub
parent f4704f1325 712614a03c
commit 5515256e53
4 changed files with 49 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
javascript/ql/lib/semmle/javascript/dataflow/Configuration.qll
View File

@@ -1145,7 +1145,8 @@ private predicate reachableFromInput(
DataFlow::Configuration cfg, PathSummary summary
) {
callInputStep(f, invk, input, nd, cfg) and
summary = PathSummary::level()
summary = PathSummary::level() and
not cfg.isLabeledBarrier(nd, summary.getEndLabel())
or
exists(DataFlow::Node mid, PathSummary oldSummary |
reachableFromInput(f, invk, input, mid, cfg, oldSummary) and

17
javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentQuery.qll
View File

@@ -32,6 +32,23 @@ class Configuration extends TaintTracking::Configuration {
or
// Concatenating with a string will in practice prevent the string `__proto__` from arising.
node instanceof StringOps::ConcatenationRoot
or
node instanceof DataFlow::ThisNode
or
// Stop at .replace() calls that likely prevent __proto__ from arising
exists(StringReplaceCall replace |
node = replace and
replace.getAReplacedString() = ["_", "p", "r", "o", "t"] and
// Replacing with "_" is likely to be exploitable
not replace.getRawReplacement().getStringValue() = "_" and
(
replace.isGlobal()
or
// Non-global replace with a non-empty string can also prevent __proto__ by
// inserting a chunk of text that doesn't fit anywhere in __proto__
not replace.getRawReplacement().getStringValue() = ""
)
)
}
override predicate isAdditionalFlowStep(

20
javascript/ql/test/query-tests/Security/CWE-915/PrototypePollutingAssignment/PrototypePollutingAssignment.expected
View File

@@ -23,6 +23,16 @@ nodes
| tst.js:45:9:45:11 | obj |
| tst.js:48:9:48:11 | obj |
| tst.js:48:9:48:11 | obj |
| tst.js:78:5:78:37 | obj[req ... ', '')] |
| tst.js:78:5:78:37 | obj[req ... ', '')] |
| tst.js:78:9:78:19 | req.query.x |
| tst.js:78:9:78:19 | req.query.x |
| tst.js:78:9:78:36 | req.que ... _', '') |
| tst.js:81:5:81:46 | obj[req ... g, '')] |
| tst.js:81:5:81:46 | obj[req ... g, '')] |
| tst.js:81:9:81:19 | req.query.x |
| tst.js:81:9:81:19 | req.query.x |
| tst.js:81:9:81:45 | req.que ... /g, '') |
edges
| tst.js:5:9:5:38 | taint | tst.js:8:12:8:16 | taint |
| tst.js:5:9:5:38 | taint | tst.js:9:12:9:16 | taint |
@@ -47,6 +57,14 @@ edges
| tst.js:33:23:33:25 | obj | tst.js:45:9:45:11 | obj |
| tst.js:33:23:33:25 | obj | tst.js:48:9:48:11 | obj |
| tst.js:33:23:33:25 | obj | tst.js:48:9:48:11 | obj |
| tst.js:78:9:78:19 | req.query.x | tst.js:78:9:78:36 | req.que ... _', '') |
| tst.js:78:9:78:19 | req.query.x | tst.js:78:9:78:36 | req.que ... _', '') |
| tst.js:78:9:78:36 | req.que ... _', '') | tst.js:78:5:78:37 | obj[req ... ', '')] |
| tst.js:78:9:78:36 | req.que ... _', '') | tst.js:78:5:78:37 | obj[req ... ', '')] |
| tst.js:81:9:81:19 | req.query.x | tst.js:81:9:81:45 | req.que ... /g, '') |
| tst.js:81:9:81:19 | req.query.x | tst.js:81:9:81:45 | req.que ... /g, '') |
| tst.js:81:9:81:45 | req.que ... /g, '') | tst.js:81:5:81:46 | obj[req ... g, '')] |
| tst.js:81:9:81:45 | req.que ... /g, '') | tst.js:81:5:81:46 | obj[req ... g, '')] |
#select
| tst.js:8:5:8:17 | object[taint] | tst.js:5:24:5:37 | req.query.data | tst.js:8:5:8:17 | object[taint] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | here |
| tst.js:9:5:9:17 | object[taint] | tst.js:5:24:5:37 | req.query.data | tst.js:9:5:9:17 | object[taint] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | here |
@@ -55,3 +73,5 @@ edges
| tst.js:39:9:39:11 | obj | tst.js:5:24:5:37 | req.query.data | tst.js:39:9:39:11 | obj | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | here |
| tst.js:45:9:45:11 | obj | tst.js:5:24:5:37 | req.query.data | tst.js:45:9:45:11 | obj | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | here |
| tst.js:48:9:48:11 | obj | tst.js:5:24:5:37 | req.query.data | tst.js:48:9:48:11 | obj | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | here |
| tst.js:78:5:78:37 | obj[req ... ', '')] | tst.js:78:9:78:19 | req.query.x | tst.js:78:5:78:37 | obj[req ... ', '')] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:78:9:78:19 | req.query.x | here |
| tst.js:81:5:81:46 | obj[req ... g, '')] | tst.js:81:9:81:19 | req.query.x | tst.js:81:5:81:46 | obj[req ... g, '')] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:81:9:81:19 | req.query.x | here |

10
javascript/ql/test/query-tests/Security/CWE-915/PrototypePollutingAssignment/tst.js
View File

@@ -71,3 +71,13 @@ class Box {
this.foo = 'bar'; // OK - 'this' won't refer to Object.prototype
}
}
app.get('/foo', (req, res) => {
let obj = {};
obj[req.query.x.replace('_', '-')].x = 'foo'; // OK
obj[req.query.x.replace('_', '')].x = 'foo'; // NOT OK
obj[req.query.x.replace(/_/g, '')].x = 'foo'; // OK
obj[req.query.x.replace(/_/g, '-')].x = 'foo'; // OK
obj[req.query.x.replace(/__proto__/g, '')].x = 'foo'; // NOT OK - "__pr__proto__oto__"
obj[req.query.x.replace('o', '0')].x = 'foo'; // OK
});