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

Merge pull request #8165 from erik-krogh/protoWrite

Browse Source 
JS: support more property writes in js/prototype-pollution-utility
This commit is contained in:
Erik Krogh Kristensen
2022-02-22 21:30:22 +01:00
committed by GitHub
parent b6b93065ff 517e17d422
commit 73f2e89f3e
4 changed files with 138 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll
View File

@@ -651,10 +651,18 @@ module DataFlow {
override string getPropertyName() { result = astNode.getArgument(1).getStringValue() }
override Node getRhs() {
exists(ObjectExpr obj | obj = astNode.getArgument(2) |
result = obj.getPropertyByName("value").getInit().flow()
exists(DataFlow::SourceNode descriptor |
descriptor = astNode.getArgument(2).flow().getALocalSource()
|
result =
descriptor
.getAPropertyWrite("get")
.getRhs()
.getALocalSource()
.(DataFlow::FunctionNode)
.getAReturn()
or
result = obj.getPropertyByName("get").getInit().flow().(DataFlow::FunctionNode).getAReturn()
result = descriptor.getAPropertyWrite("value").getRhs()
)
}

35
javascript/ql/src/Security/CWE-915/PrototypePollutingFunction.ql
View File

@@ -165,23 +165,24 @@ predicate isPotentiallyObjectPrototype(SourceNode node) {
* would typically not happen in a merge function.
*/
predicate dynamicPropWrite(DataFlow::Node base, DataFlow::Node prop, DataFlow::Node rhs) {
exists(AssignExpr write, IndexExpr index |
index = write.getLhs() and
base = index.getBase().flow() and
prop = index.getPropertyNameExpr().flow() and
rhs = write.getRhs().flow() and
not exists(prop.getStringValue()) and
not arePropertiesEnumerated(base.getALocalSource()) and
// Prune writes that are unlikely to modify Object.prototype.
// This is mainly for performance, but may block certain results due to
// not tracking out of function returns and into callbacks.
isPotentiallyObjectPrototype(base.getALocalSource()) and
// Ignore writes with an obviously safe RHS.
not exists(Expr e | e = rhs.asExpr() |
e instanceof Literal or
e instanceof ObjectExpr or
e instanceof ArrayExpr
)
exists(
DataFlow::PropWrite write // includes e.g. Object.defineProperty
|
write.getBase() = base and
write.getPropertyNameExpr().flow() = prop and
rhs = write.getRhs()
) and
not exists(prop.getStringValue()) and
not arePropertiesEnumerated(base.getALocalSource()) and
// Prune writes that are unlikely to modify Object.prototype.
// This is mainly for performance, but may block certain results due to
// not tracking out of function returns and into callbacks.
isPotentiallyObjectPrototype(base.getALocalSource()) and
// Ignore writes with an obviously safe RHS.
not exists(Expr e | e = rhs.asExpr() |
e instanceof Literal or
e instanceof ObjectExpr or
e instanceof ArrayExpr
)
}

94
javascript/ql/test/query-tests/Security/CWE-915/PrototypePollutingFunction/PrototypePollutingFunction.expected
View File

@@ -1320,6 +1320,48 @@ nodes
| tests.js:502:24:502:28 | value |
| tests.js:502:24:502:28 | value |
| tests.js:502:24:502:28 | value |
| tests.js:508:30:508:32 | dst |
| tests.js:508:30:508:32 | dst |
| tests.js:508:35:508:37 | src |
| tests.js:508:35:508:37 | src |
| tests.js:511:13:511:25 | key |
| tests.js:511:13:511:25 | key |
| tests.js:511:19:511:25 | keys[i] |
| tests.js:511:19:511:25 | keys[i] |
| tests.js:511:19:511:25 | keys[i] |
| tests.js:513:33:513:35 | dst |
| tests.js:513:33:513:35 | dst |
| tests.js:513:33:513:40 | dst[key] |
| tests.js:513:33:513:40 | dst[key] |
| tests.js:513:33:513:40 | dst[key] |
| tests.js:513:33:513:40 | dst[key] |
| tests.js:513:37:513:39 | key |
| tests.js:513:37:513:39 | key |
| tests.js:513:43:513:45 | src |
| tests.js:513:43:513:45 | src |
| tests.js:513:43:513:50 | src[key] |
| tests.js:513:43:513:50 | src[key] |
| tests.js:513:43:513:50 | src[key] |
| tests.js:513:43:513:50 | src[key] |
| tests.js:513:43:513:50 | src[key] |
| tests.js:513:47:513:49 | key |
| tests.js:513:47:513:49 | key |
| tests.js:516:32:516:34 | src |
| tests.js:516:32:516:34 | src |
| tests.js:516:32:516:39 | src[key] |
| tests.js:516:32:516:39 | src[key] |
| tests.js:516:32:516:39 | src[key] |
| tests.js:516:32:516:39 | src[key] |
| tests.js:516:32:516:39 | src[key] |
| tests.js:516:32:516:39 | src[key] |
| tests.js:516:36:516:38 | key |
| tests.js:516:36:516:38 | key |
| tests.js:517:35:517:37 | dst |
| tests.js:517:35:517:37 | dst |
| tests.js:517:35:517:37 | dst |
| tests.js:517:40:517:42 | key |
| tests.js:517:40:517:42 | key |
| tests.js:517:40:517:42 | key |
edges
| examples/PrototypePollutingFunction.js:1:16:1:18 | dst | examples/PrototypePollutingFunction.js:5:19:5:21 | dst |
| examples/PrototypePollutingFunction.js:1:16:1:18 | dst | examples/PrototypePollutingFunction.js:5:19:5:21 | dst |
@@ -2982,6 +3024,57 @@ edges
| tests.js:498:25:498:27 | key | tests.js:498:21:498:28 | src[key] |
| tests.js:500:38:500:42 | value | tests.js:494:32:494:34 | src |
| tests.js:500:38:500:42 | value | tests.js:494:32:494:34 | src |
| tests.js:508:30:508:32 | dst | tests.js:513:33:513:35 | dst |
| tests.js:508:30:508:32 | dst | tests.js:513:33:513:35 | dst |
| tests.js:508:30:508:32 | dst | tests.js:517:35:517:37 | dst |
| tests.js:508:30:508:32 | dst | tests.js:517:35:517:37 | dst |
| tests.js:508:30:508:32 | dst | tests.js:517:35:517:37 | dst |
| tests.js:508:30:508:32 | dst | tests.js:517:35:517:37 | dst |
| tests.js:508:35:508:37 | src | tests.js:513:43:513:45 | src |
| tests.js:508:35:508:37 | src | tests.js:513:43:513:45 | src |
| tests.js:508:35:508:37 | src | tests.js:516:32:516:34 | src |
| tests.js:508:35:508:37 | src | tests.js:516:32:516:34 | src |
| tests.js:511:13:511:25 | key | tests.js:513:37:513:39 | key |
| tests.js:511:13:511:25 | key | tests.js:513:37:513:39 | key |
| tests.js:511:13:511:25 | key | tests.js:513:47:513:49 | key |
| tests.js:511:13:511:25 | key | tests.js:513:47:513:49 | key |
| tests.js:511:13:511:25 | key | tests.js:516:36:516:38 | key |
| tests.js:511:13:511:25 | key | tests.js:516:36:516:38 | key |
| tests.js:511:13:511:25 | key | tests.js:517:40:517:42 | key |
| tests.js:511:13:511:25 | key | tests.js:517:40:517:42 | key |
| tests.js:511:13:511:25 | key | tests.js:517:40:517:42 | key |
| tests.js:511:13:511:25 | key | tests.js:517:40:517:42 | key |
| tests.js:511:19:511:25 | keys[i] | tests.js:511:13:511:25 | key |
| tests.js:511:19:511:25 | keys[i] | tests.js:511:13:511:25 | key |
| tests.js:511:19:511:25 | keys[i] | tests.js:511:13:511:25 | key |
| tests.js:511:19:511:25 | keys[i] | tests.js:511:13:511:25 | key |
| tests.js:513:33:513:35 | dst | tests.js:513:33:513:40 | dst[key] |
| tests.js:513:33:513:35 | dst | tests.js:513:33:513:40 | dst[key] |
| tests.js:513:33:513:40 | dst[key] | tests.js:508:30:508:32 | dst |
| tests.js:513:33:513:40 | dst[key] | tests.js:508:30:508:32 | dst |
| tests.js:513:33:513:40 | dst[key] | tests.js:508:30:508:32 | dst |
| tests.js:513:33:513:40 | dst[key] | tests.js:508:30:508:32 | dst |
| tests.js:513:37:513:39 | key | tests.js:513:33:513:40 | dst[key] |
| tests.js:513:37:513:39 | key | tests.js:513:33:513:40 | dst[key] |
| tests.js:513:43:513:45 | src | tests.js:513:43:513:50 | src[key] |
| tests.js:513:43:513:45 | src | tests.js:513:43:513:50 | src[key] |
| tests.js:513:43:513:50 | src[key] | tests.js:508:35:508:37 | src |
| tests.js:513:43:513:50 | src[key] | tests.js:508:35:508:37 | src |
| tests.js:513:43:513:50 | src[key] | tests.js:508:35:508:37 | src |
| tests.js:513:43:513:50 | src[key] | tests.js:508:35:508:37 | src |
| tests.js:513:43:513:50 | src[key] | tests.js:508:35:508:37 | src |
| tests.js:513:43:513:50 | src[key] | tests.js:508:35:508:37 | src |
| tests.js:513:47:513:49 | key | tests.js:513:43:513:50 | src[key] |
| tests.js:513:47:513:49 | key | tests.js:513:43:513:50 | src[key] |
| tests.js:516:32:516:34 | src | tests.js:516:32:516:39 | src[key] |
| tests.js:516:32:516:34 | src | tests.js:516:32:516:39 | src[key] |
| tests.js:516:32:516:34 | src | tests.js:516:32:516:39 | src[key] |
| tests.js:516:32:516:34 | src | tests.js:516:32:516:39 | src[key] |
| tests.js:516:32:516:39 | src[key] | tests.js:516:32:516:39 | src[key] |
| tests.js:516:36:516:38 | key | tests.js:516:32:516:39 | src[key] |
| tests.js:516:36:516:38 | key | tests.js:516:32:516:39 | src[key] |
| tests.js:516:36:516:38 | key | tests.js:516:32:516:39 | src[key] |
| tests.js:516:36:516:38 | key | tests.js:516:32:516:39 | src[key] |
#select
| examples/PrototypePollutingFunction.js:7:13:7:15 | dst | examples/PrototypePollutingFunction.js:2:14:2:16 | key | examples/PrototypePollutingFunction.js:7:13:7:15 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | examples/PrototypePollutingFunction.js:2:21:2:23 | src | src | examples/PrototypePollutingFunction.js:7:13:7:15 | dst | dst |
| path-assignment.js:15:13:15:18 | target | path-assignment.js:8:19:8:25 | keys[i] | path-assignment.js:15:13:15:18 | target | The property chain $@ is recursively assigned to $@ without guarding against prototype pollution. | path-assignment.js:8:19:8:25 | keys[i] | here | path-assignment.js:15:13:15:18 | target | target |
@@ -3010,3 +3103,4 @@ edges
| tests.js:467:30:467:32 | dst | tests.js:460:25:460:27 | key | tests.js:467:30:467:32 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | tests.js:460:12:460:14 | src | src | tests.js:467:30:467:32 | dst | dst |
| tests.js:477:13:477:15 | dst | tests.js:473:25:473:27 | key | tests.js:477:13:477:15 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | tests.js:473:12:473:14 | src | src | tests.js:477:13:477:15 | dst | dst |
| tests.js:489:13:489:15 | dst | tests.js:484:14:484:16 | key | tests.js:489:13:489:15 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | tests.js:484:21:484:23 | src | src | tests.js:489:13:489:15 | dst | dst |
| tests.js:517:35:517:37 | dst | tests.js:511:19:511:25 | keys[i] | tests.js:517:35:517:37 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | tests.js:509:28:509:30 | src | src | tests.js:517:35:517:37 | dst | dst |

15
javascript/ql/test/query-tests/Security/CWE-915/PrototypePollutingFunction/tests.js
View File

@@ -503,3 +503,18 @@ function copyPlainObject2(dst, src) {
}
}
}
function usingDefineProperty(dst, src) {
let keys = Object.keys(src);
for (let i = 0; i < keys.length; ++i) {
let key = keys[i];
if (dst[key]) {
usingDefineProperty(dst[key], src[key]);
} else {
var descriptor = {};
descriptor.value = src[key];
Object.defineProperty(dst, key, descriptor); // NOT OK
}
}
}