Skip to content

JS: Address some code that weren't affecting any query result #8422

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Mar 14, 2022
Original file line number Diff line number Diff line change
@@ -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.
18 changes: 12 additions & 6 deletions javascript/ql/lib/semmle/javascript/DefUse.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -265,24 +266,28 @@ 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.
*/
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())
)
}

/** Holds if `nd` is a pseudo-definition and the result is one of its inputs. */
private SsaDefinition getAPseudoDefinitionInput(SsaDefinition nd) {
/**
* 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()
}

/**
* 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 |
Expand All @@ -296,8 +301,9 @@ 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.
*/
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1718,5 +1718,5 @@ module DataFlow {
import TypeTracking
import internal.FunctionWrapperSteps

predicate localTaintStep = TaintTracking::localTaintStep/2;
deprecated predicate localTaintStep = TaintTracking::localTaintStep/2;
}
Original file line number Diff line number Diff line change
Expand Up @@ -429,9 +429,10 @@ 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]
predicate stringStep(DataFlow::Node pred, DataFlow::Node succ) {
deprecated predicate stringStep(DataFlow::Node pred, DataFlow::Node succ) {
stringConcatenationStep(pred, succ) or
stringManipulationStep(pred, succ)
}
Expand Down Expand Up @@ -1242,8 +1243,9 @@ 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.
*/
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)
}
Expand Down
17 changes: 12 additions & 5 deletions javascript/ql/lib/semmle/javascript/security/dataflow/DOM.qll
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,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: Alias for isDocumentUrl */
deprecated predicate isDocumentURL = isDocumentUrl/1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 5 additions & 2 deletions javascript/ql/src/Declarations/Definitions.qll
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import javascript

/** An identifier appearing in a defining position. */
class DefiningIdentifier extends Identifier {
/**
* DEPRECATED: Use `SsaDefinition` from `SSA.qll` instead.
* An identifier appearing in a defining position.
*/
deprecated class DefiningIdentifier extends Identifier {
DefiningIdentifier() {
this instanceof VarDecl or
exists(Assignment assgn | this = assgn.getLhs()) or
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

17 changes: 0 additions & 17 deletions javascript/ql/test/library-tests/Classes/ClassFlow.qll

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

3 changes: 0 additions & 3 deletions javascript/ql/test/library-tests/Classes/FieldInits.qll

This file was deleted.

3 changes: 0 additions & 3 deletions javascript/ql/test/library-tests/Classes/Fields.qll

This file was deleted.

This file was deleted.

3 changes: 0 additions & 3 deletions javascript/ql/test/library-tests/Classes/MethodNames.qll

This file was deleted.

3 changes: 0 additions & 3 deletions javascript/ql/test/library-tests/Classes/NewTargetExpr.qll

This file was deleted.

6 changes: 0 additions & 6 deletions javascript/ql/test/library-tests/Classes/PrivateField.qll

This file was deleted.

3 changes: 0 additions & 3 deletions javascript/ql/test/library-tests/Classes/StaticMethods.qll

This file was deleted.

3 changes: 0 additions & 3 deletions javascript/ql/test/library-tests/Classes/SuperExpr.qll

This file was deleted.

This file was deleted.

3 changes: 0 additions & 3 deletions javascript/ql/test/library-tests/Classes/getAMember.qll

This file was deleted.

65 changes: 30 additions & 35 deletions javascript/ql/test/library-tests/DefUse/DefUsePair.expected
Original file line number Diff line number Diff line change
@@ -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 |
4 changes: 2 additions & 2 deletions javascript/ql/test/library-tests/DefUse/DefUsePair.ql
Original file line number Diff line number Diff line change
@@ -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