hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-16 16:53:25 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'main' into js/move-cors-query-from-experimental

Browse Source
This commit is contained in:
Napalys Klicius
2025-09-05 12:11:09 +02:00
committed by GitHub
parent d8c4d6deb4 e719dd912d
commit b2feaaceea
106 changed files with 14432 additions and 8745 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
javascript/ql/lib/semmle/javascript/dataflow/FlowSummary.qll
View File

@@ -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()`:

45
javascript/ql/lib/semmle/javascript/dataflow/internal/BarrierGuards.qll
View File

@@ -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)
)
}
/**

4
javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll
View File

@@ -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()

2
javascript/ql/lib/semmle/javascript/dataflow/internal/VariableCapture.qll
View File

@@ -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
}

7
javascript/ql/lib/semmle/javascript/frameworks/Babel.qll
View File

@@ -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. */

2
javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll
View File

@@ -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.

6
javascript/ql/lib/semmle/javascript/internal/flow_summaries/Arrays.qll
View File

@@ -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" }

2
javascript/ql/lib/semmle/javascript/internal/flow_summaries/AsyncAwait.qll
View File

@@ -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"`.

2
javascript/ql/lib/semmle/javascript/internal/flow_summaries/Generators.qll
View File

@@ -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

2
javascript/ql/lib/semmle/javascript/internal/flow_summaries/Iterators.qll
View File

@@ -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

2
javascript/ql/lib/semmle/javascript/internal/flow_summaries/Maps.qll
View File

@@ -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

2
javascript/ql/lib/semmle/javascript/internal/flow_summaries/Promises.qll
View File

@@ -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

2
javascript/ql/lib/semmle/javascript/internal/flow_summaries/Sets.qll
View File

@@ -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

4
javascript/ql/lib/semmle/javascript/internal/flow_summaries/Strings.qll
View File

@@ -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 {

2
javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll
View File

@@ -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

5
javascript/ql/lib/semmle/javascript/security/dataflow/RemotePropertyInjectionQuery.qll
View File

@@ -10,6 +10,7 @@
import javascript
import RemotePropertyInjectionCustomizations::RemotePropertyInjection
private import semmle.javascript.DynamicPropertyAccess
/**
* A taint-tracking configuration for reasoning about remote property injection.
@@ -24,6 +25,10 @@ module RemotePropertyInjectionConfig implements DataFlow::ConfigSig {
node = StringConcatenation::getRoot(any(ConstantString str).flow())
}
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
node1 = node2.(EnumeratedPropName).getSourceObject()
}
predicate observeDiffInformedIncrementalMode() { any() }
}

4
javascript/ql/src/Declarations/UnreachableMethodOverloads.ql
View File

@@ -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())

4
javascript/ql/src/change-notes/2025-08-27-remote-property-injection-update.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The query `js/remote-property-injection` now detects property injection vulnerabilities through object enumeration patterns such as `Object.keys()`.

6
javascript/ql/test/library-tests/FlowSummary/test.ql
View File

@@ -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) }
}

28
javascript/ql/test/query-tests/Security/CWE-400/RemovePropertyInjection/RemotePropertyInjection.expected → javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/RemotePropertyInjection.expected
View File

@@ -3,6 +3,7 @@
| tst.js:13:15:13:18 | prop | tst.js:8:28:8:51 | req.que ... trolled | tst.js:13:15:13:18 | prop | A property name to write to depends on a $@. | tst.js:8:28:8:51 | req.que ... trolled | user-provided value |
| tst.js:14:31:14:34 | prop | tst.js:8:28:8:51 | req.que ... trolled | tst.js:14:31:14:34 | prop | A property name to write to depends on a $@. | tst.js:8:28:8:51 | req.que ... trolled | user-provided value |
| tst.js:16:10:16:13 | prop | tst.js:8:28:8:51 | req.que ... trolled | tst.js:16:10:16:13 | prop | A property name to write to depends on a $@. | tst.js:8:28:8:51 | req.que ... trolled | user-provided value |
| tst.js:22:10:22:12 | key | tst.js:20:14:20:21 | req.body | tst.js:22:10:22:12 | key | A property name to write to depends on a $@. | tst.js:20:14:20:21 | req.body | user-provided value |
| tstNonExpr.js:8:17:8:23 | userVal | tstNonExpr.js:5:17:5:23 | req.url | tstNonExpr.js:8:17:8:23 | userVal | A header name depends on a $@. | tstNonExpr.js:5:17:5:23 | req.url | user-provided value |
edges
| tst.js:8:6:8:9 | prop | tst.js:9:8:9:11 | prop | provenance | |
@@ -11,11 +12,13 @@ edges
| tst.js:8:6:8:9 | prop | tst.js:16:10:16:13 | prop | provenance | |
| tst.js:8:13:8:52 | myCoolL ... rolled) | tst.js:8:6:8:9 | prop | provenance | |
| tst.js:8:28:8:51 | req.que ... trolled | tst.js:8:13:8:52 | myCoolL ... rolled) | provenance | |
| tst.js:8:28:8:51 | req.que ... trolled | tst.js:21:25:21:25 | x | provenance | |
| tst.js:21:25:21:25 | x | tst.js:22:15:22:15 | x | provenance | |
| tst.js:22:6:22:11 | result | tst.js:23:9:23:14 | result | provenance | |
| tst.js:22:15:22:15 | x | tst.js:22:6:22:11 | result | provenance | |
| tst.js:23:9:23:14 | result | tst.js:23:9:23:42 | result. ... length) | provenance | |
| tst.js:8:28:8:51 | req.que ... trolled | tst.js:27:25:27:25 | x | provenance | |
| tst.js:20:14:20:21 | req.body | tst.js:21:3:21:5 | key | provenance | Config |
| tst.js:21:3:21:5 | key | tst.js:22:10:22:12 | key | provenance | |
| tst.js:27:25:27:25 | x | tst.js:28:15:28:15 | x | provenance | |
| tst.js:28:6:28:11 | result | tst.js:29:9:29:14 | result | provenance | |
| tst.js:28:15:28:15 | x | tst.js:28:6:28:11 | result | provenance | |
| tst.js:29:9:29:14 | result | tst.js:29:9:29:42 | result. ... length) | provenance | |
| tstNonExpr.js:5:7:5:13 | userVal | tstNonExpr.js:8:17:8:23 | userVal | provenance | |
| tstNonExpr.js:5:17:5:23 | req.url | tstNonExpr.js:5:7:5:13 | userVal | provenance | |
nodes
@@ -26,13 +29,16 @@ nodes
| tst.js:13:15:13:18 | prop | semmle.label | prop |
| tst.js:14:31:14:34 | prop | semmle.label | prop |
| tst.js:16:10:16:13 | prop | semmle.label | prop |
| tst.js:21:25:21:25 | x | semmle.label | x |
| tst.js:22:6:22:11 | result | semmle.label | result |
| tst.js:22:15:22:15 | x | semmle.label | x |
| tst.js:23:9:23:14 | result | semmle.label | result |
| tst.js:23:9:23:42 | result. ... length) | semmle.label | result. ... length) |
| tst.js:20:14:20:21 | req.body | semmle.label | req.body |
| tst.js:21:3:21:5 | key | semmle.label | key |
| tst.js:22:10:22:12 | key | semmle.label | key |
| tst.js:27:25:27:25 | x | semmle.label | x |
| tst.js:28:6:28:11 | result | semmle.label | result |
| tst.js:28:15:28:15 | x | semmle.label | x |
| tst.js:29:9:29:14 | result | semmle.label | result |
| tst.js:29:9:29:42 | result. ... length) | semmle.label | result. ... length) |
| tstNonExpr.js:5:7:5:13 | userVal | semmle.label | userVal |
| tstNonExpr.js:5:17:5:23 | req.url | semmle.label | req.url |
| tstNonExpr.js:8:17:8:23 | userVal | semmle.label | userVal |
subpaths
| tst.js:8:28:8:51 | req.que ... trolled | tst.js:21:25:21:25 | x | tst.js:23:9:23:42 | result. ... length) | tst.js:8:13:8:52 | myCoolL ... rolled) |
| tst.js:8:28:8:51 | req.que ... trolled | tst.js:27:25:27:25 | x | tst.js:29:9:29:42 | result. ... length) | tst.js:8:13:8:52 | myCoolL ... rolled) |

0
javascript/ql/test/query-tests/Security/CWE-400/RemovePropertyInjection/RemotePropertyInjection.qlref → javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/RemotePropertyInjection.qlref
View File

9
javascript/ql/test/query-tests/Security/CWE-400/RemovePropertyInjection/tst.js → javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/tst.js
View File

@@ -16,10 +16,15 @@ app.get('/user/:id', function(req, res) {
headers[prop] = 42; // $ Alert
res.set(headers);
myCoolLocalFct[req.query.x](); // OK - flagged by method name injection
Object.keys(req.body).forEach( // $ Source
key => {
myObj[key] = 42; // $ Alert
}
);
});
function myCoolLocalFct(x) {
var result = x;
return result.substring(0, result.length);
}
}

0
javascript/ql/test/query-tests/Security/CWE-400/RemovePropertyInjection/tstNonExpr.js → javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/tstNonExpr.js
View File