Skip to content

Improve CQL Injection Query #200

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 70 commits into from
Jul 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
ac0265e
Minor readability
jeongsoolee09 Jun 17, 2025
6fd6b2a
Make definition of `ImplMethodCallApplicationServiceDefinition` more …
jeongsoolee09 Jun 17, 2025
439ad43
Add `srv.entities` as part of `EntityEntry`
jeongsoolee09 Jun 17, 2025
3b7d741
Put CQL query-relevant definitions in a query library
jeongsoolee09 Jun 17, 2025
440712f
Do some refactoring
jeongsoolee09 Jun 19, 2025
dfa8c08
Sort out EntityReference
jeongsoolee09 Jun 19, 2025
571b316
Create a new CQL injection test project and move the old one to a folder
jeongsoolee09 Jun 20, 2025
1262260
Minor numbering
jeongsoolee09 Jun 20, 2025
908a572
install `@sap/cds-dk` and bump version of `@sap/cds`
jeongsoolee09 Jun 20, 2025
a8a0bb4
Move existing CQL Injection test case to `old/`
jeongsoolee09 Jun 23, 2025
5e39be5
Refine test cases and add more cases
jeongsoolee09 Jun 23, 2025
25450a8
Finalize working draft on current CqlInjection test case
jeongsoolee09 Jun 23, 2025
0f95069
Finalize CqlInjection
jeongsoolee09 Jun 23, 2025
1e83f5a
Fix test case and code
jeongsoolee09 Jun 24, 2025
9a2e99b
Appease compiler to make SensitiveExposure pass
jeongsoolee09 Jun 24, 2025
c5c9740
Debug `isSink`
jeongsoolee09 Jun 24, 2025
89522d0
Cover all cases in the current CQL injection test
jeongsoolee09 Jun 24, 2025
43738d5
Get the query depending on the type of sink
jeongsoolee09 Jun 24, 2025
13691d4
Minor formatting and tidying up
jeongsoolee09 Jun 24, 2025
468a780
Docstrings and comments
jeongsoolee09 Jun 24, 2025
a391d34
Fix a regression in `taintedclause`
jeongsoolee09 Jun 24, 2025
39b2397
Update expected test outputs of old cqlinjection.js
jeongsoolee09 Jun 24, 2025
fd0d13a
Fix a regression in applicationserviceinstance
jeongsoolee09 Jun 24, 2025
621a319
Fix regression in SensitiveExposure
jeongsoolee09 Jun 24, 2025
a0f6d9a
Remove unneeded code
jeongsoolee09 Jun 24, 2025
7652149
Update expected results of sensitive-exposure
jeongsoolee09 Jun 24, 2025
2e56ae0
Fix numbering and remove duplicate case
jeongsoolee09 Jun 24, 2025
a13a825
Make the new CQL injection test project a proper test case and remove…
jeongsoolee09 Jun 24, 2025
8a99661
Remove unneeded comment
jeongsoolee09 Jun 24, 2025
5c1a5ba
Checkpoint
jeongsoolee09 Jun 26, 2025
a93a7b1
Add some more unit test cases
jeongsoolee09 Jun 26, 2025
d968632
Add some more cases and add FP tags
jeongsoolee09 Jun 26, 2025
fb979d2
Remove the old CQL injection test files
jeongsoolee09 Jun 26, 2025
bdfc5bd
Add FP cases that are left out in the previous commit.
jeongsoolee09 Jun 26, 2025
0004b88
Debug query according to the new test cases
jeongsoolee09 Jun 26, 2025
f0bb40c
Add expected test result of new CQL injection test
jeongsoolee09 Jun 26, 2025
8a6e11d
Update test results of sensitive-exposure
jeongsoolee09 Jun 26, 2025
18ef3b5
Update expected analysis results
jeongsoolee09 Jun 27, 2025
cd26594
Remove `CdsFacade.getNode/0`
jeongsoolee09 Jun 30, 2025
d7f5ed4
Use `getStringValue/0` in `getNamespace/0`
jeongsoolee09 Jun 30, 2025
cee86f1
Add documentation, make `CdsDb` extend `CdsDbService`, and remove `is…
jeongsoolee09 Jun 30, 2025
5d52200
Factor out sink definition in a class and associate location reportin…
jeongsoolee09 Jun 30, 2025
caac1ae
Add some comments and docstrings
jeongsoolee09 Jun 30, 2025
97d6a01
Minor documentation
jeongsoolee09 Jun 30, 2025
9c3138b
Update test result
jeongsoolee09 Jun 30, 2025
378436a
Tidy up code
jeongsoolee09 Jun 30, 2025
689e00a
Refine the start and end of the second and third steps
jeongsoolee09 Jul 1, 2025
806e615
Explicitly use argument index in `CdsTransaction.getContextObject/0`
jeongsoolee09 Jul 1, 2025
dc42b54
Improve writing of a QLDoc
jeongsoolee09 Jul 1, 2025
28ae679
Explicitly state the safe/unsafe reason and add some documentation
jeongsoolee09 Jul 2, 2025
a3efdd7
Add documentation to `CdsTransaction`
jeongsoolee09 Jul 2, 2025
c05e8a6
Remove duplicate logic introduced by `exists(VarRef ... )`
jeongsoolee09 Jul 2, 2025
c3c882f
Update expected results of CQL injection query
jeongsoolee09 Jul 2, 2025
9101ff3
Remove all test cases that include `entries`
jeongsoolee09 Jul 3, 2025
a763874
Remove propagation steps for INSERT and UPSERT
jeongsoolee09 Jul 3, 2025
3ae7055
Update expected Code Scanning Results
jeongsoolee09 Jul 3, 2025
3b073eb
Remove related logic that detects query parameters
jeongsoolee09 Jul 3, 2025
4e77ab2
Bring back test cases with `entries`
jeongsoolee09 Jul 3, 2025
7a2db7b
Update expected result of CQL injection query
jeongsoolee09 Jul 3, 2025
d286f51
Flip `UNSAFE` label to `SAFE` and fix/remove impossible cases
jeongsoolee09 Jul 3, 2025
0d4942f
Remove additional logic to detect `entries` cases and remove the case…
jeongsoolee09 Jul 3, 2025
66ef528
Update expected result of Code Scanning
jeongsoolee09 Jul 3, 2025
565dde2
Fix a regression where the alert was not made if a child CQL clause i…
jeongsoolee09 Jul 3, 2025
71ff926
Fix wrong SAFE labels and shift comment location
jeongsoolee09 Jul 3, 2025
16c6157
Remove safe cases from expected results
jeongsoolee09 Jul 8, 2025
d26d930
Minor relocation of a label
jeongsoolee09 Jul 8, 2025
375a26a
Mark the entire shortcut method call as primary location
jeongsoolee09 Jul 8, 2025
f95efde
Change the non-clickable part of alert message
jeongsoolee09 Jul 8, 2025
5adcff5
Update expected results of code scanning
jeongsoolee09 Jul 8, 2025
6bfec53
Remove temporary internal test cases from expected results
jeongsoolee09 Jul 8, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/javascript.sarif.expected

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,203 @@
import javascript
import semmle.javascript.security.dataflow.SqlInjectionCustomizations
import advanced_security.javascript.frameworks.cap.CQL
import advanced_security.javascript.frameworks.cap.RemoteFlowSources
import advanced_security.javascript.frameworks.cap.dataflow.FlowSteps

abstract class CqlInjectionSink extends DataFlow::Node {
/**
* Gets the data flow node that represents the query being run, for
* accurate reporting.
*/
abstract DataFlow::Node getQuery();
}

/**
* A CQL clause parameterized with a string concatentation expression.
*/
class CqlClauseWithStringConcatParameter instanceof CqlClause {
CqlClauseWithStringConcatParameter() {
exists(DataFlow::Node queryParameter |
queryParameter = this.getArgument().flow() and
exists(StringConcatenation::getAnOperand(queryParameter))
)
}

Location getLocation() { result = super.getLocation() }

string toString() { result = super.toString() }
}

/**
* An await expression that has as its operand a CQL clause that includes a
* string concatenation operation.
*/
class AwaitCqlClauseWithStringConcatParameter extends CqlInjectionSink {
DataFlow::Node queryParameter;
DataFlow::Node query;
CqlClauseWithStringConcatParameter cqlClauseWithStringConcat;
CqlClause finalAncestorCqlClauseOfCqlClauseWithStringConcat;

AwaitCqlClauseWithStringConcatParameter() {
exists(AwaitExpr await |
this = await.flow() and
await.getOperand() = finalAncestorCqlClauseOfCqlClauseWithStringConcat.asExpr() and
finalAncestorCqlClauseOfCqlClauseWithStringConcat =
cqlClauseWithStringConcat.(CqlClause).getFinalClause()
)
}

override DataFlow::Node getQuery() {
result = finalAncestorCqlClauseOfCqlClauseWithStringConcat.flow()
}
}

/**
* The first argument passed to the call to `cds.run`, `cds.db.run`, or `srv.run`
* whose value is a CQL query object that includes a string concatenation. e.g.
* ``` javascript
* // 1. CQN object constructed from Fluent API
* const query = SELECT.from`Entity1`.where("ID=" + id);
* cds.run(query);
*
* // 2. CQN object parsed from a string
* const query = cds.parse.cql("SELECT * from Entity1 where ID =" + id);
* cds.run(query);
*
* // 3. An unparsed CQL string (only valid in old versions of CAP)
* const query = "SELECT * from Entity1 where ID =" + id;
* Service2.run(query);
* ```
* The `getQuery/0` member predicate gets the `query` argument of the above calls
* to `run`.
*/
class StringConcatParameterOfCqlRunMethodQueryArgument extends CqlInjectionSink {
CqlRunMethodCall cqlRunMethodCall;

StringConcatParameterOfCqlRunMethodQueryArgument() {
this = cqlRunMethodCall.getAQueryParameter()
}

override DataFlow::Node getQuery() { result = this }
}

/**
* A CQL shortcut method call (`read`, `create`, ...) parameterized with a string
* concatenation expression. e.g.
* ``` javascript
* cds.read("Entity1").where(`ID=${id}`); // Notice the surrounding parentheses!
* cds.update("Entity1").set("col1 = col1" + amount).where("col1 = " + id);
* cds.delete("Entity1").where("ID =" + id);
* ```
*/
class CqlShortcutMethodCallWithStringConcat instanceof CqlShortcutMethodCall {
DataFlow::Node stringConcatParameter;

CqlShortcutMethodCallWithStringConcat() {
stringConcatParameter = super.getAQueryParameter() and
exists(StringConcatenation::getAnOperand(stringConcatParameter))
}

Location getLocation() { result = super.getLocation() }

string toString() { result = super.toString() }

DataFlow::Node getStringConcatParameter() { result = stringConcatParameter }
}

/**
* A string concatenation expression included in a CQL shortcut method call. e.g.
* ``` javascript
* cds.read("Entity1").where(`ID=${id}`); // Notice the surrounding parentheses!
* cds.update("Entity1").set("col1 = col1" + amount).where("col1 = " + id);
* cds.delete("Entity1").where("ID =" + id);
* ```
* This class captures the string concatenation expressions appearing above:
* 1. `ID=${id}`
* 2. `"col1 = col1" + amount`
* 3. `"col1 = " + id`
* 4. `"ID =" + id`
*/
class StringConcatParameterOfCqlShortcutMethodCall extends CqlInjectionSink {
CqlShortcutMethodCallWithStringConcat cqlShortcutMethodCallWithStringConcat;

StringConcatParameterOfCqlShortcutMethodCall() {
this = cqlShortcutMethodCallWithStringConcat.getStringConcatParameter()
}

override DataFlow::Node getQuery() {
result =
cqlShortcutMethodCallWithStringConcat.(CqlShortcutMethodCall).getFinalChainedMethodCall()
}
}

/**
* A CQL parser call (`cds.ql`, `cds.parse.cql`, ...) parameterized with a string
* conatenation expression.
*/
class CqlClauseParserCallWithStringConcat instanceof CqlClauseParserCall {
CqlClauseParserCallWithStringConcat() {
not this.getCdlString().(StringOps::Concatenation).asExpr() instanceof TemplateLiteral and
exists(StringConcatenation::getAnOperand(this.getCdlString()))
}

Location getLocation() { result = super.getLocation() }

string toString() { result = super.toString() }
}

/**
* A data flow configuration from a remote flow source to a handful of sinks that run a CQL
* query, either directly or indirectly by assembling one under the hood.
*
* The CQL injection happens if a fluent API builder (`SELECT`, `INSERT`, ...) or a
* shortcut method call (`srv.read`, `srv.create`, ...) are called with a string
* concatentation as one of its argument, which in practice can take one of its
* following forms:
*
* 1. Concatentation with a string value with the `+` operator:
* - Concatenation with a string: `"ID=" + expr`
* - Concatenation with a template literal: `` `ID=` + expr ``
* 2. Template literal that interpolates an expression in it but is not a tagged
* template literal: `` SELECT.from`Entity`.where(`ID=${expr}`) ``
*
* The second case should be distinguished from the ones that have tagged template literals
* for all of its builder calls: if the example were `` SELECT.from`Entity`.where`ID=${expr}` ``
* instead (notice the lack of parentheses around the template literal), then the `where` call
* becomes a parser call of the template literal following it and thus acts as a sanitizer.
*/
class CqlInjectionConfiguration extends TaintTracking::Configuration {
CqlInjectionConfiguration() { this = "CQL injection from untrusted data" }

override predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }

override predicate isSink(DataFlow::Node node) { node instanceof CqlInjectionSink }

override predicate isSanitizer(DataFlow::Node node) { node instanceof SqlInjection::Sanitizer }

override predicate isAdditionalTaintStep(DataFlow::Node start, DataFlow::Node end) {
/*
* 1. Given a call to a CQL parser, jump from the argument to the parser call itself.
*/

exists(CqlClauseParserCall cqlParserCall |
start = cqlParserCall.getAnArgument() and
end = cqlParserCall
)
or
/*
* 2. Jump from a query parameter to the CQL query clause itself. e.g. Given below code:
*
* ``` javascript
* await SELECT.from(Service1Entity).where("ID=" + id);
* ```
*
* This step jumps from `id` in the call to `where` to the entire SELECT clause.
*/

exists(CqlClause cqlClause |
start = cqlClause.getArgument().flow().getAPredecessor*().(StringOps::Concatenation) and
end = cqlClause.getFinalClause().flow()
)
}
}
Loading