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

make a model for hasOwnProperty calls and similar

Browse Source
This commit is contained in:
Erik Krogh Kristensen
2022-05-24 14:13:53 +02:00
parent 2a97dd9f6f
commit 82c6c22d50
6 changed files with 108 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
javascript/ql/lib/semmle/javascript/MembershipCandidates.qll
View File

@@ -229,18 +229,10 @@ module MembershipCandidate {
membersNode = inExpr.getRightOperand()
)
or
exists(MethodCallExpr hasOwn |
this = hasOwn.getArgument(0).flow() and
test = hasOwn and
hasOwn.calls(membersNode, "hasOwnProperty")
)
or
exists(DataFlow::CallNode hasOwn |
hasOwn = DataFlow::globalVarRef("Object").getAMemberCall("hasOwn")
|
hasOwn.getArgument(0).asExpr() = membersNode and
this = hasOwn.getArgument(1) and
test = hasOwn.asExpr()
exists(HasOwnPropertyCall hasOwn |
this = hasOwn.getProperty() and
test = hasOwn.asExpr() and
membersNode = hasOwn.getObject().asExpr()
)
}

32
javascript/ql/lib/semmle/javascript/StandardLibrary.qll
View File

@@ -192,3 +192,35 @@ class StringSplitCall extends DataFlow::MethodCallNode {
bindingset[i]
DataFlow::Node getASubstringRead(int i) { result = this.getAPropertyRead(i.toString()) }
}
/**
* A call to `Object.prototype.hasOwnProperty`, `Object.hasOwn`, or a library that implements
* the same functionality.
*/
class HasOwnPropertyCall extends DataFlow::Node instanceof DataFlow::CallNode {
DataFlow::Node object;
DataFlow::Node property;
HasOwnPropertyCall() {
// Make sure we handle reflective calls since libraries love to do that.
super.getCalleeNode().getALocalSource().(DataFlow::PropRead).getPropertyName() =
"hasOwnProperty" and
object = super.getReceiver() and
property = super.getArgument(0)
or
this =
[
DataFlow::globalVarRef("Object").getAMemberCall("hasOwn"), //
DataFlow::moduleImport("has").getACall(), //
LodashUnderscore::member("has").getACall()
] and
object = super.getArgument(0) and
property = super.getArgument(1)
}
/** Gets the object whose property is being checked. */
DataFlow::Node getObject() { result = object }
/** Gets the property being checked. */
DataFlow::Node getProperty() { result = property }
}

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

@@ -286,7 +286,6 @@ class PropNameTracking extends DataFlow::Configuration {
node instanceof DenyListEqualityGuard or
node instanceof AllowListEqualityGuard or
node instanceof HasOwnPropertyGuard or
node instanceof HasOwnGuard or
node instanceof InExprGuard or
node instanceof InstanceOfGuard or
node instanceof TypeofGuard or
@@ -340,32 +339,16 @@ class AllowListEqualityGuard extends DataFlow::LabeledBarrierGuardNode, ValueNod
* but the destination object generally doesn't. It is therefore only a sanitizer when
* used on the destination object.
*/
class HasOwnPropertyGuard extends DataFlow::BarrierGuardNode, CallNode {
class HasOwnPropertyGuard extends DataFlow::BarrierGuardNode instanceof HasOwnPropertyCall {
HasOwnPropertyGuard() {
// Make sure we handle reflective calls since libraries love to do that.
getCalleeNode().getALocalSource().(DataFlow::PropRead).getPropertyName() = "hasOwnProperty" and
exists(getReceiver()) and
// Try to avoid `src.hasOwnProperty` by requiring that the receiver
// does not locally have its properties enumerated. Typically there is no
// reason to enumerate the properties of the destination object.
not arePropertiesEnumerated(getReceiver().getALocalSource())
not arePropertiesEnumerated(super.getObject().getALocalSource())
}
override predicate blocks(boolean outcome, Expr e) {
e = getArgument(0).asExpr() and outcome = true
}
}
/** Sanitizer guard for calls to `Object.hasOwn(obj, prop)`. */
class HasOwnGuard extends DataFlow::BarrierGuardNode, CallNode {
HasOwnGuard() {
this = DataFlow::globalVarRef("Object").getAMemberCall("hasOwn") and
// same check as in HasOwnPropertyGuard
not arePropertiesEnumerated(getArgument(0).getALocalSource())
}
override predicate blocks(boolean outcome, Expr e) {
e = getArgument(1).asExpr() and outcome = true
e = super.getProperty().asExpr() and outcome = true
}
}

1
javascript/ql/test/library-tests/TaintBarriers/tests.expected
View File

@@ -131,7 +131,6 @@ sanitizingGuard
| tst.js:395:16:395:38 | o.hasOw ... (v.p.q) | tst.js:395:33:395:37 | v.p.q | true |
| tst.js:397:16:397:36 | o.hasOw ... ty(v.p) | tst.js:397:33:397:35 | v.p | true |
| tst.js:399:16:399:41 | o.hasOw ... "p.q"]) | tst.js:399:33:399:40 | v["p.q"] | true |
| tst.js:401:16:401:34 | Object.hasOwn(o, v) | tst.js:401:30:401:30 | o | true |
| tst.js:401:16:401:34 | Object.hasOwn(o, v) | tst.js:401:33:401:33 | v | true |
taintedSink
| tst.js:2:13:2:20 | SOURCE() | tst.js:3:10:3:10 | v |

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

@@ -1503,6 +1503,31 @@ nodes
| tests.js:559:24:559:31 | src[key] |
| tests.js:559:28:559:30 | key |
| tests.js:559:28:559:30 | key |
| tests.js:564:35:564:37 | src |
| tests.js:564:35:564:37 | src |
| tests.js:565:14:565:16 | key |
| tests.js:565:14:565:16 | key |
| tests.js:565:14:565:16 | key |
| tests.js:569:43:569:45 | src |
| tests.js:569:43:569:45 | src |
| tests.js:569:43:569:50 | src[key] |
| tests.js:569:43:569:50 | src[key] |
| tests.js:569:43:569:50 | src[key] |
| tests.js:569:43:569:50 | src[key] |
| tests.js:569:43:569:50 | src[key] |
| tests.js:571:17:571:19 | key |
| tests.js:571:17:571:19 | key |
| tests.js:571:17:571:19 | key |
| tests.js:571:24:571:26 | src |
| tests.js:571:24:571:26 | src |
| tests.js:571:24:571:31 | src[key] |
| tests.js:571:24:571:31 | src[key] |
| tests.js:571:24:571:31 | src[key] |
| tests.js:571:24:571:31 | src[key] |
| tests.js:571:24:571:31 | src[key] |
| tests.js:571:24:571:31 | src[key] |
| tests.js:571:28:571:30 | key |
| tests.js:571:28:571:30 | 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 |
@@ -3404,6 +3429,38 @@ edges
| tests.js:559:28:559:30 | key | tests.js:559:24:559:31 | src[key] |
| tests.js:559:28:559:30 | key | tests.js:559:24:559:31 | src[key] |
| tests.js:559:28:559:30 | key | tests.js:559:24:559:31 | src[key] |
| tests.js:564:35:564:37 | src | tests.js:569:43:569:45 | src |
| tests.js:564:35:564:37 | src | tests.js:569:43:569:45 | src |
| tests.js:564:35:564:37 | src | tests.js:571:24:571:26 | src |
| tests.js:564:35:564:37 | src | tests.js:571:24:571:26 | src |
| tests.js:565:14:565:16 | key | tests.js:571:17:571:19 | key |
| tests.js:565:14:565:16 | key | tests.js:571:17:571:19 | key |
| tests.js:565:14:565:16 | key | tests.js:571:17:571:19 | key |
| tests.js:565:14:565:16 | key | tests.js:571:17:571:19 | key |
| tests.js:565:14:565:16 | key | tests.js:571:17:571:19 | key |
| tests.js:565:14:565:16 | key | tests.js:571:17:571:19 | key |
| tests.js:565:14:565:16 | key | tests.js:571:17:571:19 | key |
| tests.js:565:14:565:16 | key | tests.js:571:28:571:30 | key |
| tests.js:565:14:565:16 | key | tests.js:571:28:571:30 | key |
| tests.js:565:14:565:16 | key | tests.js:571:28:571:30 | key |
| tests.js:565:14:565:16 | key | tests.js:571:28:571:30 | key |
| tests.js:569:43:569:45 | src | tests.js:569:43:569:50 | src[key] |
| tests.js:569:43:569:45 | src | tests.js:569:43:569:50 | src[key] |
| tests.js:569:43:569:50 | src[key] | tests.js:564:35:564:37 | src |
| tests.js:569:43:569:50 | src[key] | tests.js:564:35:564:37 | src |
| tests.js:569:43:569:50 | src[key] | tests.js:564:35:564:37 | src |
| tests.js:569:43:569:50 | src[key] | tests.js:564:35:564:37 | src |
| tests.js:569:43:569:50 | src[key] | tests.js:564:35:564:37 | src |
| tests.js:569:43:569:50 | src[key] | tests.js:564:35:564:37 | src |
| tests.js:571:24:571:26 | src | tests.js:571:24:571:31 | src[key] |
| tests.js:571:24:571:26 | src | tests.js:571:24:571:31 | src[key] |
| tests.js:571:24:571:26 | src | tests.js:571:24:571:31 | src[key] |
| tests.js:571:24:571:26 | src | tests.js:571:24:571:31 | src[key] |
| tests.js:571:24:571:31 | src[key] | tests.js:571:24:571:31 | src[key] |
| tests.js:571:28:571:30 | key | tests.js:571:24:571:31 | src[key] |
| tests.js:571:28:571:30 | key | tests.js:571:24:571:31 | src[key] |
| tests.js:571:28:571:30 | key | tests.js:571:24:571:31 | src[key] |
| tests.js:571:28:571:30 | key | tests.js:571:24:571:31 | 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 |

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

@@ -560,3 +560,15 @@ function copyHasOwnProperty2(dst, src) {
}
}
}
function copyHasOwnProperty3(dst, src) {
for (let key in src) {
// Guarding the recursive case by dst.hasOwnProperty (or Object.hasOwn) is safe,
// since '__proto__' and 'constructor' are not own properties of the destination object.
if (_.has(dst, key)) {
copyHasOwnProperty3(dst[key], src[key]);
} else {
dst[key] = src[key]; // OK
}
}
}