mirror of
https://github.com/github/codeql.git
synced 2025-12-16 16:53:25 +01:00
Merge pull request #20329 from michaelnebel/javascript/ql4ql
JS: Fix some Ql4Ql violations.
This commit is contained in:
@@ -17,10 +17,10 @@ private import semmle.javascript.dataflow.internal.DataFlowPrivate
|
||||
*
|
||||
* - The relevant call sites cannot be matched by the access path syntax, and require the full power of CodeQL.
|
||||
* For example, complex overloading patterns might require more local reasoning at the call site.
|
||||
* - The input/output behaviour cannot be described statically in the access path syntax, but the relevant access paths
|
||||
* - The input/output behavior cannot be described statically in the access path syntax, but the relevant access paths
|
||||
* can be generated dynamically in CodeQL, based on the usages found in the codebase.
|
||||
*
|
||||
* Subclasses should bind `this` to a unique identifier for the function being modelled. There is no special
|
||||
* Subclasses should bind `this` to a unique identifier for the function being modeled. There is no special
|
||||
* interpreation of the `this` value, it should just not clash with the `this`-value used by other classes.
|
||||
*
|
||||
* For example, this models flow through calls such as `require("my-library").myFunction()`:
|
||||
|
||||
@@ -386,34 +386,35 @@ module MakeStateBarrierGuard<
|
||||
*/
|
||||
private class BarrierGuardFunction extends FinalFunction {
|
||||
DataFlow::ParameterNode sanitizedParameter;
|
||||
BarrierGuard guard;
|
||||
boolean guardOutcome;
|
||||
FlowState state;
|
||||
int paramIndex;
|
||||
|
||||
BarrierGuardFunction() {
|
||||
barrierGuardIsRelevant(guard) and
|
||||
exists(Expr e |
|
||||
exists(Expr returnExpr |
|
||||
returnExpr = guard.asExpr()
|
||||
or
|
||||
// ad hoc support for conjunctions:
|
||||
getALogicalAndParent(guard) = returnExpr and guardOutcome = true
|
||||
or
|
||||
// ad hoc support for disjunctions:
|
||||
getALogicalOrParent(guard) = returnExpr and guardOutcome = false
|
||||
|
|
||||
exists(SsaExplicitDefinition ssa |
|
||||
ssa.getDef().getSource() = returnExpr and
|
||||
ssa.getVariable().getAUse() = this.getAReturnedExpr()
|
||||
)
|
||||
or
|
||||
returnExpr = this.getAReturnedExpr()
|
||||
exists(BarrierGuard guard |
|
||||
barrierGuardIsRelevant(guard) and
|
||||
exists(Expr e |
|
||||
exists(Expr returnExpr |
|
||||
returnExpr = guard.asExpr()
|
||||
or
|
||||
// ad hoc support for conjunctions:
|
||||
getALogicalAndParent(guard) = returnExpr and guardOutcome = true
|
||||
or
|
||||
// ad hoc support for disjunctions:
|
||||
getALogicalOrParent(guard) = returnExpr and guardOutcome = false
|
||||
|
|
||||
exists(SsaExplicitDefinition ssa |
|
||||
ssa.getDef().getSource() = returnExpr and
|
||||
ssa.getVariable().getAUse() = this.getAReturnedExpr()
|
||||
)
|
||||
or
|
||||
returnExpr = this.getAReturnedExpr()
|
||||
) and
|
||||
sanitizedParameter.flowsToExpr(e) and
|
||||
barrierGuardBlocksExpr(guard, guardOutcome, e, state)
|
||||
) and
|
||||
sanitizedParameter.flowsToExpr(e) and
|
||||
barrierGuardBlocksExpr(guard, guardOutcome, e, state)
|
||||
) and
|
||||
sanitizedParameter.getParameter() = this.getParameter(paramIndex)
|
||||
sanitizedParameter.getParameter() = this.getParameter(paramIndex)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -674,7 +674,7 @@ predicate neverSkipInPathGraph(Node node) {
|
||||
// Include the return-value expression
|
||||
node.asExpr() = any(Function f).getAReturnedExpr()
|
||||
or
|
||||
// Include calls (which may have been modelled as steps)
|
||||
// Include calls (which may have been modeled as steps)
|
||||
node.asExpr() instanceof InvokeExpr
|
||||
or
|
||||
// Include references to a variable
|
||||
@@ -1103,7 +1103,7 @@ private predicate legacyBarrier(DataFlow::Node node) {
|
||||
pragma[nomagic]
|
||||
private predicate isBlockedLegacyNode(Node node) {
|
||||
// Ignore captured variable nodes for those variables that are handled by the captured-variable library.
|
||||
// Note that some variables, such as top-level variables, are still modelled with these nodes (which will result in jump steps).
|
||||
// Note that some variables, such as top-level variables, are still modeled with these nodes (which will result in jump steps).
|
||||
exists(LocalVariable variable |
|
||||
node = TCapturedVariableNode(variable) and
|
||||
variable = any(VariableCaptureConfig::CapturedVariable v).asLocalVariable()
|
||||
|
||||
@@ -170,7 +170,7 @@ module VariableCaptureConfig implements InputSig<js::Location, js::Cfg::BasicBlo
|
||||
|
||||
predicate hasCfgNode(js::Cfg::BasicBlock bb, int i) { none() } // Overridden in subclass
|
||||
|
||||
// note: langauge-specific
|
||||
// note: language-specific
|
||||
js::DataFlow::Node getSource() { none() } // Overridden in subclass
|
||||
}
|
||||
|
||||
|
||||
@@ -141,16 +141,17 @@ module Babel {
|
||||
*/
|
||||
deprecated private class BabelRootTransformedPathExpr extends PathExpr, Expr {
|
||||
RootImportConfig plugin;
|
||||
string prefix;
|
||||
string mappedPrefix;
|
||||
string suffix;
|
||||
|
||||
BabelRootTransformedPathExpr() {
|
||||
this instanceof PathExpr and
|
||||
plugin.appliesTo(this.getTopLevel()) and
|
||||
prefix = this.getStringValue().regexpCapture("(.)/(.*)", 1) and
|
||||
suffix = this.getStringValue().regexpCapture("(.)/(.*)", 2) and
|
||||
mappedPrefix = plugin.getRoot(prefix)
|
||||
exists(string prefix |
|
||||
prefix = this.getStringValue().regexpCapture("(.)/(.*)", 1) and
|
||||
mappedPrefix = plugin.getRoot(prefix)
|
||||
)
|
||||
}
|
||||
|
||||
/** Gets the configuration that applies to this path. */
|
||||
|
||||
@@ -35,7 +35,7 @@ class Location = JS::Location;
|
||||
* Type names have form `package.type` or just `package` if referring to the package export
|
||||
* object. If `package` contains a `.` character it must be enclosed in single quotes, such as `'package'.type`.
|
||||
*
|
||||
* A type name of form `(package)` may also be used when refering to the package export object.
|
||||
* A type name of form `(package)` may also be used when referring to the package export object.
|
||||
* We allow this syntax as an alternative to the above, so models generated based on `EndpointNaming` look more consistent.
|
||||
* However, access paths are deliberately not parsed here, as we can not handle aliasing at this stage.
|
||||
* The model generator must explicitly generate the step between `(package)` and `(package).foo`, for example.
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
/**
|
||||
* Contains a summary for relevant methods on arrays.
|
||||
*
|
||||
* Note that some of Array methods are modelled in `AmbiguousCoreMethods.qll`, and `toString` is special-cased elsewhere.
|
||||
* Note that some of Array methods are modeled in `AmbiguousCoreMethods.qll`, and `toString` is special-cased elsewhere.
|
||||
*/
|
||||
|
||||
private import javascript
|
||||
@@ -60,7 +60,7 @@ private predicate isForLoopVariable(Variable v) {
|
||||
|
||||
private predicate isLikelyArrayIndex(Expr e) {
|
||||
// Require that 'e' is of type number and refers to a for-loop variable.
|
||||
// TODO: This is here to mirror the old behaviour. Experiment with turning the 'and' into an 'or'.
|
||||
// TODO: This is here to mirror the old behavior. Experiment with turning the 'and' into an 'or'.
|
||||
TTNumber() = unique(InferredType type | type = e.flow().analyze().getAType()) and
|
||||
isForLoopVariable(e.(VarAccess).getVariable())
|
||||
or
|
||||
@@ -114,7 +114,7 @@ class ArrayConstructorSummary extends SummarizedCallable {
|
||||
/**
|
||||
* A call to `join` with a separator argument.
|
||||
*
|
||||
* Calls without separators are modelled in `StringConcatenation.qll`.
|
||||
* Calls without separators are modeled in `StringConcatenation.qll`.
|
||||
*/
|
||||
class Join extends SummarizedCallable {
|
||||
Join() { this = "Array#join" }
|
||||
|
||||
@@ -8,7 +8,7 @@ private import semmle.javascript.dataflow.internal.AdditionalFlowInternal
|
||||
private import semmle.javascript.dataflow.internal.DataFlowPrivate
|
||||
|
||||
/**
|
||||
* Steps modelling flow in an `async` function.
|
||||
* Steps modeling flow in an `async` function.
|
||||
*
|
||||
* Note about promise-coercion and flattening:
|
||||
* - `await` preserves non-promise values, e.g. `await "foo"` is just `"foo"`.
|
||||
|
||||
@@ -7,7 +7,7 @@ private import semmle.javascript.dataflow.internal.DataFlowNode
|
||||
private import semmle.javascript.dataflow.internal.AdditionalFlowInternal
|
||||
|
||||
/**
|
||||
* Steps modelling flow out of a generator function:
|
||||
* Steps modeling flow out of a generator function:
|
||||
* ```js
|
||||
* function* foo() {
|
||||
* yield x; // store 'x' in the return value's IteratorElement
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/**
|
||||
* Contains flow summaries and steps modelling flow through iterators.
|
||||
* Contains flow summaries and steps modeling flow through iterators.
|
||||
*/
|
||||
|
||||
private import javascript
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/**
|
||||
* Contains flow summaries and steps modelling flow through `Map` objects.
|
||||
* Contains flow summaries and steps modeling flow through `Map` objects.
|
||||
*/
|
||||
|
||||
private import javascript
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/**
|
||||
* Contains flow summaries and steps modelling flow through `Promise` objects.
|
||||
* Contains flow summaries and steps modeling flow through `Promise` objects.
|
||||
*/
|
||||
|
||||
private import javascript
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/**
|
||||
* Contains flow summaries and steps modelling flow through `Set` objects.
|
||||
* Contains flow summaries and steps modeling flow through `Set` objects.
|
||||
*/
|
||||
|
||||
private import javascript
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/**
|
||||
* Contains flow summaries and steps modelling flow through string methods.
|
||||
* Contains flow summaries and steps modeling flow through string methods.
|
||||
*/
|
||||
|
||||
private import javascript
|
||||
@@ -73,7 +73,7 @@ class StringSplit extends SummarizedCallable {
|
||||
* These are of special significance when tracking a tainted URL suffix, such as `window.location.href`,
|
||||
* because the first element of the resulting array should not be considered tainted.
|
||||
*
|
||||
* This summary defaults to the same behaviour as the general `.split()` case, but it contains optional steps
|
||||
* This summary defaults to the same behavior as the general `.split()` case, but it contains optional steps
|
||||
* and barriers named `tainted-url-suffix` that should be activated when tracking a tainted URL suffix.
|
||||
*/
|
||||
class StringSplitHashOrQuestionMark extends SummarizedCallable {
|
||||
|
||||
@@ -76,7 +76,7 @@ module TaintedUrlSuffix {
|
||||
//
|
||||
// x [tainted-url-suffix] --> x.split('#') [array element 1] [taint]
|
||||
//
|
||||
// Technically we should also preverse tainted-url-suffix when entering the first array element of such
|
||||
// Technically we should also preserve tainted-url-suffix when entering the first array element of such
|
||||
// a split, but this mostly leads to FPs since we currently don't track if the taint has been through URI-decoding.
|
||||
// (The query/fragment parts are often URI-decoded in practice, but not the other URL parts are not)
|
||||
state1.isTaintedUrlSuffix() and
|
||||
|
||||
@@ -46,9 +46,7 @@ string getKind(MemberDeclaration m) {
|
||||
* A call-signature that originates from a MethodSignature in the AST.
|
||||
*/
|
||||
private class MethodCallSig extends Function {
|
||||
private MethodSignature signature;
|
||||
|
||||
MethodCallSig() { this = signature.getBody() }
|
||||
MethodCallSig() { this = any(MethodSignature signature).getBody() }
|
||||
|
||||
int getNumOptionalParameter() {
|
||||
result = count(Parameter p | p = this.getParameter(_) and p.isDeclaredOptional())
|
||||
|
||||
@@ -8,7 +8,7 @@ DataFlow::CallNode getACall(string name) {
|
||||
result.getCalleeNode().getALocalSource() = DataFlow::globalVarRef(name)
|
||||
}
|
||||
|
||||
module ConfigArg implements DataFlow::ConfigSig {
|
||||
module FlowConfig implements DataFlow::ConfigSig {
|
||||
predicate isSource(DataFlow::Node node) { node = getACall("source") }
|
||||
|
||||
predicate isSink(DataFlow::Node node) { node = getACall("sink").getAnArgument() }
|
||||
@@ -19,7 +19,7 @@ module ConfigArg implements DataFlow::ConfigSig {
|
||||
}
|
||||
}
|
||||
|
||||
module Configuration = DataFlow::Global<ConfigArg>;
|
||||
module Flow = DataFlow::Global<FlowConfig>;
|
||||
|
||||
class BasicBarrierGuard extends DataFlow::CallNode {
|
||||
BasicBarrierGuard() { this = getACall("isSafe") }
|
||||
@@ -32,5 +32,5 @@ class BasicBarrierGuard extends DataFlow::CallNode {
|
||||
deprecated class ConsistencyConfig extends ConsistencyConfiguration {
|
||||
ConsistencyConfig() { this = "ConsistencyConfig" }
|
||||
|
||||
override DataFlow::Node getAnAlert() { Configuration::flow(_, result) }
|
||||
override DataFlow::Node getAnAlert() { Flow::flow(_, result) }
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user