From 4c9f406e34be74ed8e2429c2255f4f0335085c89 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 6 Jan 2025 10:32:11 +0100 Subject: [PATCH 1/5] JS: Exclude some sinks in UnvalidatedDynamicMethodCall --- .../dataflow/UnvalidatedDynamicMethodCallCustomizations.qll | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll index f148ba2c96bb..e516167a30b4 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll @@ -182,7 +182,11 @@ module UnvalidatedDynamicMethodCall { exists(InvokeExpr invk | this = invk.getCallee().flow() and // don't flag invocations inside a try-catch - not invk.getASuccessor() instanceof CatchClause + not invk.getASuccessor() instanceof CatchClause and + // Filter out `foo.bar()` calls as they usually aren't interesting. + // Technically this could be reachable if preceded by `foo.bar = obj[taint]` + // but such sinks are more likely to be FPs and also slow down the query. + not invk.getCallee() instanceof DotExpr ) } From e2af19b94609ff3d6022826fb08a4bcb07c8cc9c Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 6 Jan 2025 13:17:32 +0100 Subject: [PATCH 2/5] JS: Restrict "get" step to Map objects --- .../UnvalidatedDynamicMethodCallQuery.qll | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallQuery.qll index d8b29fca9014..399d4852cc50 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallQuery.qll @@ -24,6 +24,17 @@ deprecated private class ConcreteMaybeFromProto extends MaybeFromProto { ConcreteMaybeFromProto() { this = this } } +/** Gets a data flow node referring to an instance of `Map`. */ +private DataFlow::SourceNode mapObject(DataFlow::TypeTracker t) { + t.start() and + result = DataFlow::globalVarRef("Map").getAnInstantiation() + or + exists(DataFlow::TypeTracker t2 | result = mapObject(t2).track(t2, t)) +} + +/** Gets a data flow node referring to an instance of `Map`. */ +private DataFlow::SourceNode mapObject() { result = mapObject(DataFlow::TypeTracker::end()) } + /** * A taint-tracking configuration for reasoning about unvalidated dynamic method calls. */ @@ -67,7 +78,9 @@ module UnvalidatedDynamicMethodCallConfig implements DataFlow::StateConfigSig { not PropertyInjection::hasUnsafeMethods(read.getBase().getALocalSource()) ) or - exists(DataFlow::SourceNode base, DataFlow::CallNode get | get = base.getAMethodCall("get") | + exists(DataFlow::CallNode get | + get = mapObject().getAMethodCall("get") and + get.getNumArgument() = 1 and node1 = get.getArgument(0) and node2 = get ) and From 23d7420cece6cf20765f0cee013d21704fe2f276 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 6 Jan 2025 14:27:20 +0100 Subject: [PATCH 3/5] JS: Hide default exceptional return node --- .../lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll | 2 ++ 1 file changed, 2 insertions(+) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll index 72de0f5c0458..8629288f3309 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll @@ -612,6 +612,8 @@ predicate nodeIsHidden(Node node) { or node instanceof FlowSummaryIntermediateAwaitStoreNode or + node instanceof FlowSummaryDefaultExceptionalReturn + or node instanceof CaptureNode or // Hide function expressions, as capture-flow causes them to appear in unhelpful ways From 7ccb476b1bcf80680428c5e861a446d7a669a67f Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 6 Jan 2025 14:28:58 +0100 Subject: [PATCH 4/5] JS: Restrict AP length in ExceptionXss --- .../semmle/javascript/security/dataflow/ExceptionXssQuery.qll | 2 ++ 1 file changed, 2 insertions(+) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssQuery.qll index 71603d38ecd6..009367f4f87d 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssQuery.qll @@ -153,6 +153,8 @@ module ExceptionXssConfig implements DataFlow::StateConfigSig { canThrowSensitiveInformation(node1) and node2 = getExceptionTarget(node1) } + + int accessPathLimit() { result = 1 } } /** From 0cdda871612641db645c43a19d5a438070779a53 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 6 Jan 2025 14:33:41 +0100 Subject: [PATCH 5/5] JS: Restrict AP length in prototype-polluting function --- .../ql/src/Security/CWE-915/PrototypePollutingFunction.ql | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/javascript/ql/src/Security/CWE-915/PrototypePollutingFunction.ql b/javascript/ql/src/Security/CWE-915/PrototypePollutingFunction.ql index ba7c6d177cbb..2a49f47379ce 100644 --- a/javascript/ql/src/Security/CWE-915/PrototypePollutingFunction.ql +++ b/javascript/ql/src/Security/CWE-915/PrototypePollutingFunction.ql @@ -277,6 +277,12 @@ module PropNameTrackingConfig implements DataFlow::StateConfigSig { node instanceof DataFlow::VarAccessBarrier or node = DataFlow::MakeBarrierGuard::getABarrierNode() } + + int accessPathLimit() { + // Speed up the query. For the pattern we're looking for the value rarely + // flows through any contents, apart from a capture content. + result = 1 + } } class FlowState = PropNameTrackingConfig::FlowState;