From 9cf0a94e4d1c28ce17a9764263c8039e319eb311 Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Sun, 13 Mar 2022 23:25:33 +0100 Subject: [PATCH 1/9] use some Sanitizer classes that were unused in the query code --- .../security/dataflow/DeepObjectResourceExhaustionQuery.qll | 5 +++++ .../security/dataflow/HardcodedCredentialsQuery.qll | 5 +++++ .../javascript/security/dataflow/InsecureDownloadQuery.qll | 5 +++++ .../dataflow/UnvalidatedDynamicMethodCallCustomizations.qll | 2 ++ .../security/dataflow/UnvalidatedDynamicMethodCallQuery.qll | 6 +++++- .../javascript/security/dataflow/ResourceExhaustion.qll | 5 +++++ 6 files changed, 27 insertions(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/DeepObjectResourceExhaustionQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/DeepObjectResourceExhaustionQuery.qll index 4f693a193dc..918ef0663c8 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/DeepObjectResourceExhaustionQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/DeepObjectResourceExhaustionQuery.qll @@ -26,6 +26,11 @@ class Configuration extends TaintTracking::Configuration { guard instanceof TaintedObject::SanitizerGuard } + override predicate isSanitizer(DataFlow::Node node) { + super.isSanitizer(node) or + node instanceof Sanitizer + } + override predicate isAdditionalFlowStep( DataFlow::Node src, DataFlow::Node trg, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl ) { diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedCredentialsQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedCredentialsQuery.qll index 2b84c54e5e1..b38d1908faf 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedCredentialsQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedCredentialsQuery.qll @@ -19,6 +19,11 @@ class Configuration extends DataFlow::Configuration { override predicate isSink(DataFlow::Node sink) { sink instanceof Sink } + override predicate isBarrier(DataFlow::Node node) { + super.isBarrier(node) or + node instanceof Sanitizer + } + override predicate isAdditionalFlowStep(DataFlow::Node src, DataFlow::Node trg) { exists(Base64::Encode encode | src = encode.getInput() and trg = encode.getOutput()) or diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadQuery.qll index d61c2f6a626..cf7ce75600b 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadQuery.qll @@ -31,4 +31,9 @@ class Configuration extends DataFlow::Configuration { override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { sink.(Sink).getALabel() = label } + + override predicate isBarrier(DataFlow::Node node) { + super.isBarrier(node) or + node instanceof Sanitizer + } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll index f7e875d1654..2353cf68c50 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll @@ -34,6 +34,8 @@ module UnvalidatedDynamicMethodCall { /** * A sanitizer for unvalidated dynamic method calls. + * Override the `sanitizes` predicate to specify an edge that should be sanitized. + * The `this` value is not seen as a sanitizer. */ abstract class Sanitizer extends DataFlow::Node { abstract predicate sanitizes(DataFlow::Node source, DataFlow::Node sink, DataFlow::FlowLabel lbl); diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallQuery.qll index 0733465fd62..7a0b6594194 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallQuery.qll @@ -38,7 +38,11 @@ class Configuration extends TaintTracking::Configuration { sink.(Sink).getFlowLabel() = label } - override predicate isSanitizer(DataFlow::Node nd) { super.isSanitizer(nd) } + override predicate isSanitizerEdge( + DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel lbl + ) { + any(Sanitizer s).sanitizes(pred, succ, lbl) + } override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) { guard instanceof NumberGuard or diff --git a/javascript/ql/src/experimental/semmle/javascript/security/dataflow/ResourceExhaustion.qll b/javascript/ql/src/experimental/semmle/javascript/security/dataflow/ResourceExhaustion.qll index 582df0ee071..c77ce32efbd 100644 --- a/javascript/ql/src/experimental/semmle/javascript/security/dataflow/ResourceExhaustion.qll +++ b/javascript/ql/src/experimental/semmle/javascript/security/dataflow/ResourceExhaustion.qll @@ -23,6 +23,11 @@ module ResourceExhaustion { override predicate isSink(DataFlow::Node sink) { sink instanceof Sink } + override predicate isSanitizer(DataFlow::Node node) { + super.isSanitizer(node) or + node instanceof Sanitizer + } + override predicate isAdditionalTaintStep(DataFlow::Node src, DataFlow::Node dst) { isNumericFlowStep(src, dst) or From 4fc85a791d45d931ec6dac0304e8c314eb0ba23a Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Sun, 13 Mar 2022 23:26:51 +0100 Subject: [PATCH 2/9] deprecate DefiningIdentifier, it was not used in any query --- javascript/ql/src/Declarations/Definitions.qll | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/javascript/ql/src/Declarations/Definitions.qll b/javascript/ql/src/Declarations/Definitions.qll index 2f83cb80a60..65f7ca994ac 100644 --- a/javascript/ql/src/Declarations/Definitions.qll +++ b/javascript/ql/src/Declarations/Definitions.qll @@ -1,7 +1,7 @@ import javascript -/** An identifier appearing in a defining position. */ -class DefiningIdentifier extends Identifier { +/** DEPRECATED: An identifier appearing in a defining position. */ +deprecated class DefiningIdentifier extends Identifier { DefiningIdentifier() { this instanceof VarDecl or exists(Assignment assgn | this = assgn.getLhs()) or From 5e52a710912b244bd4195b0ebc9de67a058ba04a Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Sun, 13 Mar 2022 23:28:14 +0100 Subject: [PATCH 3/9] remove test .qll files that weren't imported --- .../library-tests/Classes/AccessorMethods.qll | 3 --- .../Classes/ClassDefinition_getName.qll | 3 --- .../Classes/ClassDefinition_getSuperClass.qll | 5 ----- .../library-tests/Classes/ClassDefinitions.qll | 3 --- .../ql/test/library-tests/Classes/ClassFlow.qll | 17 ----------------- .../Classes/ClassNodeConstructor.qll | 5 ----- .../Classes/ClassNodeInstanceMethod.qll | 7 ------- .../Classes/ClassNodeStaticMethod.qll | 7 ------- .../library-tests/Classes/ComputedMethods.qll | 3 --- .../Classes/ConstructorDefinitions.qll | 3 --- .../test/library-tests/Classes/FieldInits.qll | 3 --- .../ql/test/library-tests/Classes/Fields.qll | 3 --- .../library-tests/Classes/MethodDefinitions.qll | 7 ------- .../test/library-tests/Classes/MethodNames.qll | 3 --- .../library-tests/Classes/NewTargetExpr.qll | 3 --- .../test/library-tests/Classes/PrivateField.qll | 6 ------ .../library-tests/Classes/StaticMethods.qll | 3 --- .../ql/test/library-tests/Classes/SuperExpr.qll | 3 --- .../Classes/SyntheticConstructors.qll | 3 --- .../test/library-tests/Classes/getAMember.qll | 3 --- 20 files changed, 93 deletions(-) delete mode 100644 javascript/ql/test/library-tests/Classes/AccessorMethods.qll delete mode 100644 javascript/ql/test/library-tests/Classes/ClassDefinition_getName.qll delete mode 100644 javascript/ql/test/library-tests/Classes/ClassDefinition_getSuperClass.qll delete mode 100644 javascript/ql/test/library-tests/Classes/ClassDefinitions.qll delete mode 100644 javascript/ql/test/library-tests/Classes/ClassFlow.qll delete mode 100644 javascript/ql/test/library-tests/Classes/ClassNodeConstructor.qll delete mode 100644 javascript/ql/test/library-tests/Classes/ClassNodeInstanceMethod.qll delete mode 100644 javascript/ql/test/library-tests/Classes/ClassNodeStaticMethod.qll delete mode 100644 javascript/ql/test/library-tests/Classes/ComputedMethods.qll delete mode 100644 javascript/ql/test/library-tests/Classes/ConstructorDefinitions.qll delete mode 100644 javascript/ql/test/library-tests/Classes/FieldInits.qll delete mode 100644 javascript/ql/test/library-tests/Classes/Fields.qll delete mode 100644 javascript/ql/test/library-tests/Classes/MethodDefinitions.qll delete mode 100644 javascript/ql/test/library-tests/Classes/MethodNames.qll delete mode 100644 javascript/ql/test/library-tests/Classes/NewTargetExpr.qll delete mode 100644 javascript/ql/test/library-tests/Classes/PrivateField.qll delete mode 100644 javascript/ql/test/library-tests/Classes/StaticMethods.qll delete mode 100644 javascript/ql/test/library-tests/Classes/SuperExpr.qll delete mode 100644 javascript/ql/test/library-tests/Classes/SyntheticConstructors.qll delete mode 100644 javascript/ql/test/library-tests/Classes/getAMember.qll diff --git a/javascript/ql/test/library-tests/Classes/AccessorMethods.qll b/javascript/ql/test/library-tests/Classes/AccessorMethods.qll deleted file mode 100644 index 32dca6cabc8..00000000000 --- a/javascript/ql/test/library-tests/Classes/AccessorMethods.qll +++ /dev/null @@ -1,3 +0,0 @@ -import javascript - -query predicate test_AccessorMethods(AccessorMethodDefinition amd) { any() } diff --git a/javascript/ql/test/library-tests/Classes/ClassDefinition_getName.qll b/javascript/ql/test/library-tests/Classes/ClassDefinition_getName.qll deleted file mode 100644 index 0eb8bec773c..00000000000 --- a/javascript/ql/test/library-tests/Classes/ClassDefinition_getName.qll +++ /dev/null @@ -1,3 +0,0 @@ -import javascript - -query predicate test_ClassDefinition_getName(ClassDefinition cd, string res) { res = cd.getName() } diff --git a/javascript/ql/test/library-tests/Classes/ClassDefinition_getSuperClass.qll b/javascript/ql/test/library-tests/Classes/ClassDefinition_getSuperClass.qll deleted file mode 100644 index 30bf54a496a..00000000000 --- a/javascript/ql/test/library-tests/Classes/ClassDefinition_getSuperClass.qll +++ /dev/null @@ -1,5 +0,0 @@ -import javascript - -query predicate test_ClassDefinition_getSuperClass(ClassDefinition cd, Expr res) { - res = cd.getSuperClass() -} diff --git a/javascript/ql/test/library-tests/Classes/ClassDefinitions.qll b/javascript/ql/test/library-tests/Classes/ClassDefinitions.qll deleted file mode 100644 index 94150fd65a1..00000000000 --- a/javascript/ql/test/library-tests/Classes/ClassDefinitions.qll +++ /dev/null @@ -1,3 +0,0 @@ -import javascript - -query predicate test_ClassDefinitions(ClassDefinition cd) { any() } diff --git a/javascript/ql/test/library-tests/Classes/ClassFlow.qll b/javascript/ql/test/library-tests/Classes/ClassFlow.qll deleted file mode 100644 index dec2475702c..00000000000 --- a/javascript/ql/test/library-tests/Classes/ClassFlow.qll +++ /dev/null @@ -1,17 +0,0 @@ -import javascript - -class Configuration extends DataFlow::Configuration { - Configuration() { this = "ClassDataFlowTestingConfig" } - - override predicate isSource(DataFlow::Node source) { - source.getEnclosingExpr().(StringLiteral).getValue().toLowerCase() = "source" - } - - override predicate isSink(DataFlow::Node sink) { - any(DataFlow::CallNode call | call.getCalleeName() = "sink").getAnArgument() = sink - } -} - -query predicate dataflow(DataFlow::Node pred, DataFlow::Node succ) { - any(Configuration c).hasFlow(pred, succ) -} diff --git a/javascript/ql/test/library-tests/Classes/ClassNodeConstructor.qll b/javascript/ql/test/library-tests/Classes/ClassNodeConstructor.qll deleted file mode 100644 index da1abf434c9..00000000000 --- a/javascript/ql/test/library-tests/Classes/ClassNodeConstructor.qll +++ /dev/null @@ -1,5 +0,0 @@ -import javascript - -query predicate test_ClassNodeConstructor(DataFlow::ClassNode class_, DataFlow::FunctionNode res) { - res = class_.getConstructor() -} diff --git a/javascript/ql/test/library-tests/Classes/ClassNodeInstanceMethod.qll b/javascript/ql/test/library-tests/Classes/ClassNodeInstanceMethod.qll deleted file mode 100644 index a631a7b1168..00000000000 --- a/javascript/ql/test/library-tests/Classes/ClassNodeInstanceMethod.qll +++ /dev/null @@ -1,7 +0,0 @@ -import javascript - -query predicate test_ClassNodeInstanceMethod( - DataFlow::ClassNode class_, string name, DataFlow::FunctionNode res -) { - res = class_.getInstanceMethod(name) -} diff --git a/javascript/ql/test/library-tests/Classes/ClassNodeStaticMethod.qll b/javascript/ql/test/library-tests/Classes/ClassNodeStaticMethod.qll deleted file mode 100644 index cdfd7e0617a..00000000000 --- a/javascript/ql/test/library-tests/Classes/ClassNodeStaticMethod.qll +++ /dev/null @@ -1,7 +0,0 @@ -import javascript - -query predicate test_ClassNodeStaticMethod( - DataFlow::ClassNode class_, string name, DataFlow::FunctionNode res -) { - res = class_.getStaticMethod(name) -} diff --git a/javascript/ql/test/library-tests/Classes/ComputedMethods.qll b/javascript/ql/test/library-tests/Classes/ComputedMethods.qll deleted file mode 100644 index 19087903766..00000000000 --- a/javascript/ql/test/library-tests/Classes/ComputedMethods.qll +++ /dev/null @@ -1,3 +0,0 @@ -import javascript - -query predicate test_ComputedMethods(MethodDefinition md) { md.isComputed() } diff --git a/javascript/ql/test/library-tests/Classes/ConstructorDefinitions.qll b/javascript/ql/test/library-tests/Classes/ConstructorDefinitions.qll deleted file mode 100644 index e4478cee98e..00000000000 --- a/javascript/ql/test/library-tests/Classes/ConstructorDefinitions.qll +++ /dev/null @@ -1,3 +0,0 @@ -import javascript - -query predicate test_ConstructorDefinitions(ConstructorDefinition cd) { any() } diff --git a/javascript/ql/test/library-tests/Classes/FieldInits.qll b/javascript/ql/test/library-tests/Classes/FieldInits.qll deleted file mode 100644 index 5ea97665207..00000000000 --- a/javascript/ql/test/library-tests/Classes/FieldInits.qll +++ /dev/null @@ -1,3 +0,0 @@ -import javascript - -query predicate test_FieldInits(FieldDefinition field, Expr res) { res = field.getInit() } diff --git a/javascript/ql/test/library-tests/Classes/Fields.qll b/javascript/ql/test/library-tests/Classes/Fields.qll deleted file mode 100644 index fcb73440a49..00000000000 --- a/javascript/ql/test/library-tests/Classes/Fields.qll +++ /dev/null @@ -1,3 +0,0 @@ -import javascript - -query predicate test_Fields(FieldDefinition field, Expr res) { res = field.getNameExpr() } diff --git a/javascript/ql/test/library-tests/Classes/MethodDefinitions.qll b/javascript/ql/test/library-tests/Classes/MethodDefinitions.qll deleted file mode 100644 index 89a93217160..00000000000 --- a/javascript/ql/test/library-tests/Classes/MethodDefinitions.qll +++ /dev/null @@ -1,7 +0,0 @@ -import javascript - -query predicate test_MethodDefinitions( - MethodDefinition md, Expr res0, FunctionExpr res1, ClassDefinition res2 -) { - res0 = md.getNameExpr() and res1 = md.getBody() and res2 = md.getDeclaringClass() -} diff --git a/javascript/ql/test/library-tests/Classes/MethodNames.qll b/javascript/ql/test/library-tests/Classes/MethodNames.qll deleted file mode 100644 index d956696ac71..00000000000 --- a/javascript/ql/test/library-tests/Classes/MethodNames.qll +++ /dev/null @@ -1,3 +0,0 @@ -import javascript - -query predicate test_MethodNames(MethodDefinition md, string res) { res = md.getName() } diff --git a/javascript/ql/test/library-tests/Classes/NewTargetExpr.qll b/javascript/ql/test/library-tests/Classes/NewTargetExpr.qll deleted file mode 100644 index ea35cb80d11..00000000000 --- a/javascript/ql/test/library-tests/Classes/NewTargetExpr.qll +++ /dev/null @@ -1,3 +0,0 @@ -import javascript - -query predicate test_NewTargetExpr(NewTargetExpr e) { any() } diff --git a/javascript/ql/test/library-tests/Classes/PrivateField.qll b/javascript/ql/test/library-tests/Classes/PrivateField.qll deleted file mode 100644 index d4f5d3d4678..00000000000 --- a/javascript/ql/test/library-tests/Classes/PrivateField.qll +++ /dev/null @@ -1,6 +0,0 @@ -import javascript - -query string getAccessModifier(DataFlow::PropRef ref, Expr prop) { - prop = ref.getPropertyNameExpr() and - if ref.isPrivateField() then result = "Private" else result = "Public" -} diff --git a/javascript/ql/test/library-tests/Classes/StaticMethods.qll b/javascript/ql/test/library-tests/Classes/StaticMethods.qll deleted file mode 100644 index ba0f01f54d7..00000000000 --- a/javascript/ql/test/library-tests/Classes/StaticMethods.qll +++ /dev/null @@ -1,3 +0,0 @@ -import javascript - -query predicate test_StaticMethods(MethodDefinition md) { md.isStatic() } diff --git a/javascript/ql/test/library-tests/Classes/SuperExpr.qll b/javascript/ql/test/library-tests/Classes/SuperExpr.qll deleted file mode 100644 index b5ca8b95073..00000000000 --- a/javascript/ql/test/library-tests/Classes/SuperExpr.qll +++ /dev/null @@ -1,3 +0,0 @@ -import javascript - -query predicate test_SuperExpr(SuperExpr s) { any() } diff --git a/javascript/ql/test/library-tests/Classes/SyntheticConstructors.qll b/javascript/ql/test/library-tests/Classes/SyntheticConstructors.qll deleted file mode 100644 index 2f379fe8af1..00000000000 --- a/javascript/ql/test/library-tests/Classes/SyntheticConstructors.qll +++ /dev/null @@ -1,3 +0,0 @@ -import javascript - -query predicate test_SyntheticConstructors(ConstructorDefinition cd) { cd.isSynthetic() } diff --git a/javascript/ql/test/library-tests/Classes/getAMember.qll b/javascript/ql/test/library-tests/Classes/getAMember.qll deleted file mode 100644 index 0ba3084bcd8..00000000000 --- a/javascript/ql/test/library-tests/Classes/getAMember.qll +++ /dev/null @@ -1,3 +0,0 @@ -import javascript - -query predicate test_getAMember(ClassDefinition c, MemberDeclaration res) { res = c.getAMember() } From c0a63beec1b0300c17f1dd0cfb070b8e77e7508b Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Sun, 13 Mar 2022 23:32:07 +0100 Subject: [PATCH 4/9] deprecate unused document predicates in DOM.qll --- .../semmle/javascript/security/dataflow/DOM.qll | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/DOM.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/DOM.qll index dd88867f4f0..46cabb36151 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/DOM.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/DOM.qll @@ -29,15 +29,22 @@ predicate isLocation(Expr e) { } /** + * DEPRECATED: Use DOM::documentRef() instead. * Gets a reference to the 'document' object. */ -DataFlow::SourceNode document() { result = DOM::documentRef() } +deprecated DataFlow::SourceNode document() { result = DOM::documentRef() } -/** Holds if `e` could refer to the `document` object. */ -predicate isDocument(Expr e) { DOM::documentRef().flowsToExpr(e) } +/** + * DEPRECATED: Use DOM::documentRef() instead. + * Holds if `e` could refer to the `document` object. + */ +deprecated predicate isDocument(Expr e) { DOM::documentRef().flowsToExpr(e) } -/** Holds if `e` could refer to the document URL. */ -predicate isDocumentURL(Expr e) { e.flow() = DOM::locationSource() } +/** + * DEPRECATED: Use DOM::locationSource() instead. + * Holds if `e` could refer to the document URL. + */ +deprecated predicate isDocumentURL(Expr e) { e.flow() = DOM::locationSource() } /** * DEPRECATED. In most cases, a sanitizer based on this predicate can be removed, as From cc231fef4caf2dbd2e93b218df2352231c78e781 Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Sun, 13 Mar 2022 23:35:18 +0100 Subject: [PATCH 5/9] deprecate some unused predicate in DefUse.qll --- javascript/ql/lib/semmle/javascript/DefUse.qll | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/DefUse.qll b/javascript/ql/lib/semmle/javascript/DefUse.qll index 6fa0b438370..02e8a70c589 100644 --- a/javascript/ql/lib/semmle/javascript/DefUse.qll +++ b/javascript/ql/lib/semmle/javascript/DefUse.qll @@ -266,7 +266,7 @@ predicate definitionReaches(Variable v, VarDef def, VarUse use) { * Holds if the definition of local variable `v` in `def` reaches `use` along some control flow path * without crossing another definition of `v`. */ -predicate localDefinitionReaches(LocalVariable v, VarDef def, VarUse use) { +deprecated predicate localDefinitionReaches(LocalVariable v, VarDef def, VarUse use) { exists(SsaExplicitDefinition ssa | ssa.defines(def, v) and ssa = getAPseudoDefinitionInput*(use.getSsaVariable().getDefinition()) @@ -274,7 +274,7 @@ predicate localDefinitionReaches(LocalVariable v, VarDef def, VarUse use) { } /** Holds if `nd` is a pseudo-definition and the result is one of its inputs. */ -private SsaDefinition getAPseudoDefinitionInput(SsaDefinition nd) { +deprecated private SsaDefinition getAPseudoDefinitionInput(SsaDefinition nd) { result = nd.(SsaPseudoDefinition).getAnInput() } @@ -297,7 +297,7 @@ private int nextDefAfter(BasicBlock bb, Variable v, int i, VarDef d) { * This is the case if there is a path from `earlier` to `later` that does not cross * another definition of `v`. */ -predicate localDefinitionOverwrites(LocalVariable v, VarDef earlier, VarDef later) { +deprecated predicate localDefinitionOverwrites(LocalVariable v, VarDef earlier, VarDef later) { exists(BasicBlock bb, int i, int next | next = nextDefAfter(bb, v, i, earlier) | bb.defAt(next, v, later) or From 8f86b067e7d8eda843227520554f796ae328959f Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Sun, 13 Mar 2022 23:50:31 +0100 Subject: [PATCH 6/9] deprecate the unused localTaintStep and stringStep predicates --- javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll | 2 +- .../ql/lib/semmle/javascript/dataflow/TaintTracking.qll | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll b/javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll index 5a4b1ac3f46..aa2a151a94c 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll @@ -1718,5 +1718,5 @@ module DataFlow { import TypeTracking import internal.FunctionWrapperSteps - predicate localTaintStep = TaintTracking::localTaintStep/2; + deprecated predicate localTaintStep = TaintTracking::localTaintStep/2; } diff --git a/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll b/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll index 63a678f22ba..4ebe2072987 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll @@ -431,7 +431,7 @@ module TaintTracking { * Holds if `pred -> succ` is a taint propagating data flow edge through a string operation. */ pragma[inline] - predicate stringStep(DataFlow::Node pred, DataFlow::Node succ) { + deprecated predicate stringStep(DataFlow::Node pred, DataFlow::Node succ) { stringConcatenationStep(pred, succ) or stringManipulationStep(pred, succ) } @@ -1243,7 +1243,7 @@ module TaintTracking { /** * Holds if taint propagates from `pred` to `succ` in one local (intra-procedural) step. */ - predicate localTaintStep(DataFlow::Node pred, DataFlow::Node succ) { + deprecated predicate localTaintStep(DataFlow::Node pred, DataFlow::Node succ) { DataFlow::localFlowStep(pred, succ) or sharedTaintStep(pred, succ) } From 54760081dc9e233a2d5be966921a10e85046067a Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Mon, 14 Mar 2022 10:10:38 +0100 Subject: [PATCH 7/9] add pointers to the qldoc of deprecated predicates --- javascript/ql/lib/semmle/javascript/DefUse.qll | 7 ++++++- .../ql/lib/semmle/javascript/dataflow/TaintTracking.qll | 2 ++ javascript/ql/src/Declarations/Definitions.qll | 5 ++++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/DefUse.qll b/javascript/ql/lib/semmle/javascript/DefUse.qll index 02e8a70c589..ce8b37c78ef 100644 --- a/javascript/ql/lib/semmle/javascript/DefUse.qll +++ b/javascript/ql/lib/semmle/javascript/DefUse.qll @@ -265,6 +265,7 @@ predicate definitionReaches(Variable v, VarDef def, VarUse use) { /** * Holds if the definition of local variable `v` in `def` reaches `use` along some control flow path * without crossing another definition of `v`. + * DEPRECATED: Use the `SSA.qll` library instead. */ deprecated predicate localDefinitionReaches(LocalVariable v, VarDef def, VarUse use) { exists(SsaExplicitDefinition ssa | @@ -273,7 +274,10 @@ deprecated predicate localDefinitionReaches(LocalVariable v, VarDef def, VarUse ) } -/** Holds if `nd` is a pseudo-definition and the result is one of its inputs. */ +/** + * Holds if `nd` is a pseudo-definition and the result is one of its inputs. + * DEPRECATED: Use the `SSA.qll` library instead. + */ deprecated private SsaDefinition getAPseudoDefinitionInput(SsaDefinition nd) { result = nd.(SsaPseudoDefinition).getAnInput() } @@ -296,6 +300,7 @@ private int nextDefAfter(BasicBlock bb, Variable v, int i, VarDef d) { * * This is the case if there is a path from `earlier` to `later` that does not cross * another definition of `v`. + * DEPRECATED: Use the `SSA.qll` library instead. */ deprecated predicate localDefinitionOverwrites(LocalVariable v, VarDef earlier, VarDef later) { exists(BasicBlock bb, int i, int next | next = nextDefAfter(bb, v, i, earlier) | diff --git a/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll b/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll index 4ebe2072987..9b935122370 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll @@ -429,6 +429,7 @@ module TaintTracking { /** * Holds if `pred -> succ` is a taint propagating data flow edge through a string operation. + * DEPRECATED: Use `stringConcatenationStep` and `stringManipulationStep` instead. */ pragma[inline] deprecated predicate stringStep(DataFlow::Node pred, DataFlow::Node succ) { @@ -1242,6 +1243,7 @@ module TaintTracking { /** * Holds if taint propagates from `pred` to `succ` in one local (intra-procedural) step. + * DEPRECATED: Use `TaintTracking::sharedTaintStep` and `DataFlow::Node::getALocalSource()` instead. */ deprecated predicate localTaintStep(DataFlow::Node pred, DataFlow::Node succ) { DataFlow::localFlowStep(pred, succ) or diff --git a/javascript/ql/src/Declarations/Definitions.qll b/javascript/ql/src/Declarations/Definitions.qll index 65f7ca994ac..a7034ad5093 100644 --- a/javascript/ql/src/Declarations/Definitions.qll +++ b/javascript/ql/src/Declarations/Definitions.qll @@ -1,6 +1,9 @@ import javascript -/** DEPRECATED: An identifier appearing in a defining position. */ +/** + * DEPRECATED: Use `SsaDefinition` from `SSA.qll` instead. + * An identifier appearing in a defining position. + */ deprecated class DefiningIdentifier extends Identifier { DefiningIdentifier() { this instanceof VarDecl or From 6d66ea42534617a80082d5b6677030e1d4652afc Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Mon, 14 Mar 2022 10:14:15 +0100 Subject: [PATCH 8/9] also deprecate the definitionReaches predicate, it was only used in a test --- .../ql/lib/semmle/javascript/DefUse.qll | 5 +- .../library-tests/DefUse/DefUsePair.expected | 65 +++++++++---------- .../test/library-tests/DefUse/DefUsePair.ql | 4 +- 3 files changed, 35 insertions(+), 39 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/DefUse.qll b/javascript/ql/lib/semmle/javascript/DefUse.qll index ce8b37c78ef..a36c682d766 100644 --- a/javascript/ql/lib/semmle/javascript/DefUse.qll +++ b/javascript/ql/lib/semmle/javascript/DefUse.qll @@ -249,8 +249,9 @@ class VarUse extends ControlFlowNode, @varref { /** * Holds if the definition of `v` in `def` reaches `use` along some control flow path * without crossing another definition of `v`. + * DEPRECATED: Use the `SSA.qll` library instead. */ -predicate definitionReaches(Variable v, VarDef def, VarUse use) { +deprecated predicate definitionReaches(Variable v, VarDef def, VarUse use) { v = use.getVariable() and exists(BasicBlock bb, int i, int next | next = nextDefAfter(bb, v, i, def) | exists(int j | j in [i + 1 .. next - 1] | bb.useAt(j, v, use)) @@ -286,7 +287,7 @@ deprecated private SsaDefinition getAPseudoDefinitionInput(SsaDefinition nd) { * Holds if `d` is a definition of `v` at index `i` in `bb`, and the result is the next index * in `bb` after `i` at which the same variable is defined, or `bb.length()` if there is none. */ -private int nextDefAfter(BasicBlock bb, Variable v, int i, VarDef d) { +deprecated private int nextDefAfter(BasicBlock bb, Variable v, int i, VarDef d) { bb.defAt(i, v, d) and result = min(int jj | diff --git a/javascript/ql/test/library-tests/DefUse/DefUsePair.expected b/javascript/ql/test/library-tests/DefUse/DefUsePair.expected index 6712137f691..caaf440a778 100644 --- a/javascript/ql/test/library-tests/DefUse/DefUsePair.expected +++ b/javascript/ql/test/library-tests/DefUse/DefUsePair.expected @@ -1,35 +1,30 @@ -| classes.js:1:1:2:1 | class Foo {\\n} | classes.js:4:1:4:3 | Foo | -| classes.js:7:5:8:5 | class L ... {\\n } | classes.js:10:5:10:12 | LocalFoo | -| es2015.js:1:10:1:11 | fn | es2015.js:2:3:2:4 | fn | -| es2015.js:5:16:5:16 | i | es2015.js:5:32:5:32 | i | -| es2015.js:5:16:5:16 | i | es2015.js:5:34:5:34 | i | -| es2015modules.js:1:10:1:12 | foo | es2015modules.js:4:3:4:5 | foo | -| es2015modules.js:1:15:1:24 | bar as baz | es2015modules.js:6:3:6:5 | baz | -| es2015modules.js:10:10:10:13 | quux | es2015modules.js:7:3:7:6 | quux | -| es2015modules.js:15:17:15:17 | f | es2015modules.js:12:1:12:1 | f | -| es2015modules.js:16:25:16:25 | g | es2015modules.js:13:1:13:1 | g | -| fundecls.js:3:12:3:12 | f | fundecls.js:4:3:4:3 | f | -| fundecls.js:9:10:9:10 | s | fundecls.js:7:1:7:1 | s | -| fundecls.js:12:12:12:12 | f | fundecls.js:10:3:10:3 | f | -| fundecls.js:18:12:18:12 | f | fundecls.js:17:3:17:3 | f | -| fundecls.js:23:12:23:12 | f | fundecls.js:24:3:24:3 | f | -| fundecls.js:34:12:34:12 | f | fundecls.js:35:3:35:3 | f | -| fundecls.js:39:11:39:11 | x | fundecls.js:40:7:40:7 | x | -| fundecls.js:41:14:41:14 | f | fundecls.js:45:3:45:3 | f | -| fundecls.js:43:14:43:14 | f | fundecls.js:45:3:45:3 | f | -| fundecls.js:48:11:48:11 | x | fundecls.js:50:7:50:7 | x | -| tst.js:1:12:1:12 | o | tst.js:3:12:3:12 | o | -| tst.js:1:12:1:12 | o | tst.js:5:16:5:16 | o | -| tst.js:2:9:2:14 | y = 23 | tst.js:8:17:8:17 | y | -| tst.js:2:17:2:21 | i = 0 | tst.js:4:5:4:5 | i | -| tst.js:2:17:2:21 | i = 0 | tst.js:7:6:7:6 | i | -| tst.js:4:3:4:5 | ++i | tst.js:4:5:4:5 | i | -| tst.js:4:3:4:5 | ++i | tst.js:7:6:7:6 | i | -| tst.js:5:11:5:11 | z | tst.js:6:7:6:7 | z | -| tst.js:5:11:5:11 | z | tst.js:8:14:8:14 | z | -| tst.js:7:4:7:6 | --i | tst.js:7:6:7:6 | i | -| tst.js:12:2:12:7 | x = 42 | tst.js:14:9:14:9 | x | -| tst.js:19:11:19:11 | x | tst.js:18:9:18:9 | x | -| tst.js:23:6:23:23 | {a = b, c = d} = e | tst.js:24:2:24:2 | a | -| tst.js:23:6:23:23 | {a = b, c = d} = e | tst.js:24:6:24:6 | c | -| tst.js:26:11:26:11 | a | tst.js:27:2:27:2 | a | +| classes.js:7:5:8:5 | def@7:5 | classes.js:10:5:10:12 | LocalFoo | +| es2015.js:1:10:1:11 | def@1:10 | es2015.js:2:3:2:4 | fn | +| es2015.js:5:16:5:16 | def@5:16 | es2015.js:5:32:5:32 | i | +| es2015.js:5:16:5:16 | def@5:16 | es2015.js:5:34:5:34 | i | +| es2015modules.js:1:10:1:12 | def@1:10 | es2015modules.js:4:3:4:5 | foo | +| es2015modules.js:1:15:1:24 | def@1:15 | es2015modules.js:6:3:6:5 | baz | +| es2015modules.js:10:10:10:13 | def@10:10 | es2015modules.js:7:3:7:6 | quux | +| es2015modules.js:15:17:15:17 | def@15:17 | es2015modules.js:12:1:12:1 | f | +| es2015modules.js:16:25:16:25 | def@16:25 | es2015modules.js:13:1:13:1 | g | +| fundecls.js:3:12:3:12 | def@3:12 | fundecls.js:4:3:4:3 | f | +| fundecls.js:12:12:12:12 | def@12:12 | fundecls.js:10:3:10:3 | f | +| fundecls.js:18:12:18:12 | def@18:12 | fundecls.js:17:3:17:3 | f | +| fundecls.js:23:12:23:12 | def@23:12 | fundecls.js:24:3:24:3 | f | +| fundecls.js:27:2:27:2 | implicitInit@27:2 | fundecls.js:28:3:28:3 | f | +| fundecls.js:34:12:34:12 | def@34:12 | fundecls.js:35:3:35:3 | f | +| fundecls.js:39:11:39:11 | def@39:11 | fundecls.js:40:7:40:7 | x | +| fundecls.js:45:3:45:3 | phi@45:3 | fundecls.js:45:3:45:3 | f | +| fundecls.js:48:11:48:11 | def@48:11 | fundecls.js:50:7:50:7 | x | +| tst.js:1:12:1:12 | def@1:12 | tst.js:3:12:3:12 | o | +| tst.js:1:12:1:12 | def@1:12 | tst.js:5:16:5:16 | o | +| tst.js:2:9:2:14 | def@2:9 | tst.js:8:17:8:17 | y | +| tst.js:3:2:3:2 | phi@3:2 | tst.js:4:5:4:5 | i | +| tst.js:5:2:5:2 | phi@5:2 | tst.js:7:6:7:6 | i | +| tst.js:5:2:5:2 | phi@5:2 | tst.js:8:14:8:14 | z | +| tst.js:5:11:5:11 | def@5:11 | tst.js:6:7:6:7 | z | +| tst.js:12:2:12:7 | def@12:2 | tst.js:14:9:14:9 | x | +| tst.js:19:11:19:11 | def@19:11 | tst.js:18:9:18:9 | x | +| tst.js:23:6:23:23 | def@23:6 | tst.js:24:2:24:2 | a | +| tst.js:23:6:23:23 | def@23:6 | tst.js:24:6:24:6 | c | +| tst.js:26:11:26:11 | def@26:11 | tst.js:27:2:27:2 | a | diff --git a/javascript/ql/test/library-tests/DefUse/DefUsePair.ql b/javascript/ql/test/library-tests/DefUse/DefUsePair.ql index bf1ce96f8b9..ae5dff0de31 100644 --- a/javascript/ql/test/library-tests/DefUse/DefUsePair.ql +++ b/javascript/ql/test/library-tests/DefUse/DefUsePair.ql @@ -1,5 +1,5 @@ import javascript -from VarDef def, VarUse use -where definitionReaches(_, def, use) +from SsaVariable def, VarUse use +where def.getAUse() = use select def, use From c06336480c94c21300a7a7efe34a3ae4e7629dc0 Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Mon, 14 Mar 2022 11:41:53 +0100 Subject: [PATCH 9/9] add change note --- .../ql/lib/change-notes/2022-02-14-deprecated-predicates.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 javascript/ql/lib/change-notes/2022-02-14-deprecated-predicates.md diff --git a/javascript/ql/lib/change-notes/2022-02-14-deprecated-predicates.md b/javascript/ql/lib/change-notes/2022-02-14-deprecated-predicates.md new file mode 100644 index 00000000000..1b8bc1d53e8 --- /dev/null +++ b/javascript/ql/lib/change-notes/2022-02-14-deprecated-predicates.md @@ -0,0 +1,5 @@ +--- +category: deprecated +--- +* Some predicates from `DefUse.qll`, `DataFlow.qll`, `TaintTracking.qll`, `DOM.qll`, `Definitions.qll` that weren't used by any query have been deprecated. + The documentation for each predicate points to an alternative.