Skip to content

Commit 87f29ad

Browse files
authored
Merge branch 'main' into redsun82/rules_rust
2 parents 179021e + 9a8cb1a commit 87f29ad

File tree

82 files changed

+4535
-261
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

82 files changed

+4535
-261
lines changed

.github/codeql/codeql-config.yml

+2
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,6 @@ paths-ignore:
1010
- '/javascript/ql/test'
1111
- '/javascript/ql/integration-tests'
1212
- '/javascript/extractor/tests'
13+
- '/javascript/extractor/parser-tests'
14+
- '/javascript/ql/src/'
1315
- '/rust/ql'
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: feature
3+
---
4+
* Added `Node.asUncertainDefinition` and `Node.asCertainDefinition` to the `DataFlow::Node` class for querying whether a definition overwrites the entire destination buffer.

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

+69-3
Original file line numberDiff line numberDiff line change
@@ -313,13 +313,79 @@ class Node extends TIRDataFlowNode {
313313
* `n.asExpr() instanceof IncrementOperation` since the result of evaluating
314314
* the expression `x++` is passed to `sink`.
315315
*/
316-
Expr asDefinition() {
317-
exists(StoreInstruction store |
316+
Expr asDefinition() { result = this.asDefinition(_) }
317+
318+
/**
319+
* Gets the definition associated with this node, if any.
320+
*
321+
* For example, consider the following example
322+
* ```cpp
323+
* int x = 42; // 1
324+
* x = 34; // 2
325+
* ++x; // 3
326+
* x++; // 4
327+
* x += 1; // 5
328+
* int y = x += 2; // 6
329+
* ```
330+
* - For (1) the result is `42`.
331+
* - For (2) the result is `x = 34`.
332+
* - For (3) the result is `++x`.
333+
* - For (4) the result is `x++`.
334+
* - For (5) the result is `x += 1`.
335+
* - For (6) there are two results:
336+
* - For the definition generated by `x += 2` the result is `x += 2`
337+
* - For the definition generated by `int y = ...` the result is
338+
* also `x += 2`.
339+
*
340+
* For assignments, `node.asDefinition(_)` and `node.asExpr()` will both exist
341+
* for the same dataflow node. However, for expression such as `x++` that
342+
* both write to `x` and read the current value of `x`, `node.asDefinition(_)`
343+
* will give the node corresponding to the value after the increment, and
344+
* `node.asExpr()` will give the node corresponding to the value before the
345+
* increment. For an example of this, consider the following:
346+
*
347+
* ```cpp
348+
* sink(x++);
349+
* ```
350+
* in the above program, there will not be flow from a node `n` such that
351+
* `n.asDefinition(_) instanceof IncrementOperation` to the argument of `sink`
352+
* since the value passed to `sink` is the value before to the increment.
353+
* However, there will be dataflow from a node `n` such that
354+
* `n.asExpr() instanceof IncrementOperation` since the result of evaluating
355+
* the expression `x++` is passed to `sink`.
356+
*
357+
* If `uncertain = false` then the definition is guaranteed to overwrite
358+
* the entire buffer pointed to by the destination address of the definition.
359+
* Otherwise, `uncertain = true`.
360+
*
361+
* For example, the write `int x; x = 42;` is guaranteed to overwrite all the
362+
* bytes allocated to `x`, while the assignment `int p[10]; p[3] = 42;` has
363+
* `uncertain = true` since the write will not overwrite the entire buffer
364+
* pointed to by `p`.
365+
*/
366+
Expr asDefinition(boolean uncertain) {
367+
exists(StoreInstruction store, Ssa::DefinitionExt def |
318368
store = this.asInstruction() and
319-
result = asDefinitionImpl(store)
369+
result = asDefinitionImpl(store) and
370+
Ssa::defToNode(this, def, _, _, _, _) and
371+
if def.isCertain() then uncertain = false else uncertain = true
320372
)
321373
}
322374

375+
/**
376+
* Gets the definition associated with this node, if this node is a certain definition.
377+
*
378+
* See `Node.asDefinition/1` for a description of certain and uncertain definitions.
379+
*/
380+
Expr asCertainDefinition() { result = this.asDefinition(false) }
381+
382+
/**
383+
* Gets the definition associated with this node, if this node is an uncertain definition.
384+
*
385+
* See `Node.asDefinition/1` for a description of certain and uncertain definitions.
386+
*/
387+
Expr asUncertainDefinition() { result = this.asDefinition(true) }
388+
323389
/**
324390
* Gets the indirect definition at a given indirection corresponding to this
325391
* node, if any.

cpp/ql/src/JPL_C/LOC-3/Rule 17/BasicIntTypes.ql

+7-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,11 @@
1212
import cpp
1313

1414
predicate allowedTypedefs(TypedefType t) {
15-
t.getName() = ["I64", "U64", "I32", "U32", "I16", "U16", "I8", "U8", "F64", "F32"]
15+
t.getName() =
16+
[
17+
"I64", "U64", "I32", "U32", "I16", "U16", "I8", "U8", "F64", "F32", "int64_t", "uint64_t",
18+
"int32_t", "uint32_t", "int16_t", "uint16_t", "int8_t", "uint8_t"
19+
]
1620
}
1721

1822
/**
@@ -46,6 +50,8 @@ from Declaration d, Type usedType
4650
where
4751
usedType = getAUsedType*(getAnImmediateUsedType(d)) and
4852
problematic(usedType) and
53+
// Allow uses of boolean types where defined by the language.
54+
not usedType instanceof BoolType and
4955
// Ignore violations for which we do not have a valid location.
5056
not d.getLocation() instanceof UnknownLocation
5157
select d,

cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ module Config implements DataFlow::ConfigSig {
3737
predicate isBarrier(DataFlow::Node node) {
3838
isSink(node) and node.asExpr().getUnspecifiedType() instanceof ArithmeticType
3939
or
40-
node.asInstruction().(StoreInstruction).getResultType() instanceof ArithmeticType
40+
node.asCertainDefinition().getUnspecifiedType() instanceof ArithmeticType
4141
}
4242
}
4343

cpp/ql/src/Security/CWE/CWE-114/UncontrolledProcessOperation.ql

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ module Config implements DataFlow::ConfigSig {
3737
predicate isBarrier(DataFlow::Node node) {
3838
isSink(node) and node.asExpr().getUnspecifiedType() instanceof ArithmeticType
3939
or
40-
node.asInstruction().(StoreInstruction).getResultType() instanceof ArithmeticType
40+
node.asCertainDefinition().getUnspecifiedType() instanceof ArithmeticType
4141
}
4242
}
4343

cpp/ql/src/Security/CWE/CWE-134/UncontrolledFormatString.ql

+1-1
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ module Config implements DataFlow::ConfigSig {
4242
predicate isBarrier(DataFlow::Node node) {
4343
isSink(node) and isArithmeticNonCharType(node.asExpr().getUnspecifiedType())
4444
or
45-
isArithmeticNonCharType(node.asInstruction().(StoreInstruction).getResultType())
45+
isArithmeticNonCharType(node.asCertainDefinition().getUnspecifiedType())
4646
}
4747
}
4848

cpp/ql/src/Security/CWE/CWE-170/ImproperNullTerminationTainted.ql

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ private module Config implements DataFlow::ConfigSig {
3737
predicate isBarrier(DataFlow::Node node) {
3838
isSink(node) and node.asExpr().getUnspecifiedType() instanceof ArithmeticType
3939
or
40-
node.asInstruction().(StoreInstruction).getResultType() instanceof ArithmeticType
40+
node.asCertainDefinition().getUnspecifiedType() instanceof ArithmeticType
4141
or
4242
mayAddNullTerminator(_, node.asIndirectExpr())
4343
}

cpp/ql/src/Security/CWE/CWE-190/ArithmeticTainted.ql

+4-2
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,11 @@ module Config implements DataFlow::ConfigSig {
7575
predicate isSink(DataFlow::Node sink) { isSink(sink, _, _) }
7676

7777
predicate isBarrier(DataFlow::Node node) {
78-
exists(StoreInstruction store | store = node.asInstruction() |
78+
exists(StoreInstruction store, Expr e |
79+
store = node.asInstruction() and e = node.asCertainDefinition()
80+
|
7981
// Block flow to "likely small expressions"
80-
bounded(store.getSourceValue().getUnconvertedResultExpression())
82+
bounded(e)
8183
or
8284
// Block flow to "small types"
8385
store.getResultType().getUnspecifiedType().(IntegralType).getSize() <= 1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The query "Use of basic integral type" (`cpp/jpl-c/basic-int-types`) no longer produces alerts for the standard fixed width integer types (`int8_t`, `uint8_t`, etc.), and the `_Bool` and `bool` types.
Original file line numberDiff line numberDiff line change
@@ -1,3 +1 @@
11
| test.c:6:26:6:26 | x | x uses the basic integral type unsigned char rather than a typedef with size and signedness. |
2-
| test.c:7:20:7:20 | x | x uses the basic integral type unsigned char rather than a typedef with size and signedness. |
3-
| test.c:10:16:10:20 | test7 | test7 uses the basic integral type unsigned char rather than a typedef with size and signedness. |

csharp/ql/src/Useless code/IntGetHashCode.ql

+8-1
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,12 @@ import semmle.code.csharp.frameworks.System
1616
from MethodCall mc, IntegralType t
1717
where
1818
mc.getTarget() instanceof GetHashCodeMethod and
19-
t = mc.getQualifier().getType()
19+
t = mc.getQualifier().getType() and
20+
(
21+
t instanceof ByteType or
22+
t instanceof SByteType or
23+
t instanceof ShortType or
24+
t instanceof UShortType or
25+
t instanceof IntType
26+
)
2027
select mc, "Calling GetHashCode() on type " + t.toStringWithTypes() + " is redundant."
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Increase query precision for `cs/useless-gethashcode-call` by not flagging calls to `GetHashCode` on `uint`, `long` and `ulong`.

csharp/ql/src/codeql-suites/csharp-ccr.qls

+1
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,4 @@
1010
- cs/call-to-object-tostring
1111
- cs/local-not-disposed
1212
- cs/constant-condition
13+
- cs/useless-gethashcode-call

csharp/ql/test/query-tests/Useless Code/IntGetHashCode/IntGetHashCode.cs

+7-7
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,16 @@ class IntGetHashCode
33
void Test()
44
{
55
// These are all bad:
6+
default(int).GetHashCode(); // $ Alert
7+
default(short).GetHashCode(); // $ Alert
8+
default(ushort).GetHashCode(); // $ Alert
9+
default(byte).GetHashCode(); // $ Alert
10+
default(sbyte).GetHashCode(); // $ Alert
11+
12+
// These are all good:
613
default(uint).GetHashCode();
7-
default(int).GetHashCode();
814
default(long).GetHashCode();
915
default(ulong).GetHashCode();
10-
default(short).GetHashCode();
11-
default(ushort).GetHashCode();
12-
default(byte).GetHashCode();
13-
default(sbyte).GetHashCode();
14-
15-
// These are all good:
1616
default(double).GetHashCode();
1717
default(float).GetHashCode();
1818
default(char).GetHashCode();
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
1-
| IntGetHashCode.cs:6:9:6:35 | call to method GetHashCode | Calling GetHashCode() on type uint is redundant. |
2-
| IntGetHashCode.cs:7:9:7:34 | call to method GetHashCode | Calling GetHashCode() on type int is redundant. |
3-
| IntGetHashCode.cs:8:9:8:35 | call to method GetHashCode | Calling GetHashCode() on type long is redundant. |
4-
| IntGetHashCode.cs:9:9:9:36 | call to method GetHashCode | Calling GetHashCode() on type ulong is redundant. |
5-
| IntGetHashCode.cs:10:9:10:36 | call to method GetHashCode | Calling GetHashCode() on type short is redundant. |
6-
| IntGetHashCode.cs:11:9:11:37 | call to method GetHashCode | Calling GetHashCode() on type ushort is redundant. |
7-
| IntGetHashCode.cs:12:9:12:35 | call to method GetHashCode | Calling GetHashCode() on type byte is redundant. |
8-
| IntGetHashCode.cs:13:9:13:36 | call to method GetHashCode | Calling GetHashCode() on type sbyte is redundant. |
1+
| IntGetHashCode.cs:6:9:6:34 | call to method GetHashCode | Calling GetHashCode() on type int is redundant. |
2+
| IntGetHashCode.cs:7:9:7:36 | call to method GetHashCode | Calling GetHashCode() on type short is redundant. |
3+
| IntGetHashCode.cs:8:9:8:37 | call to method GetHashCode | Calling GetHashCode() on type ushort is redundant. |
4+
| IntGetHashCode.cs:9:9:9:35 | call to method GetHashCode | Calling GetHashCode() on type byte is redundant. |
5+
| IntGetHashCode.cs:10:9:10:36 | call to method GetHashCode | Calling GetHashCode() on type sbyte is redundant. |
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1-
Useless code/IntGetHashCode.ql
1+
query: Useless code/IntGetHashCode.ql
2+
postprocess: utils/test/InlineExpectationsTestQuery.ql
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added taint-steps for `unescape()`.

javascript/ql/lib/semmle/javascript/Promises.qll

+12-24
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import javascript
66
private import dataflow.internal.StepSummary
7+
private import semmle.javascript.dataflow.internal.FlowSteps
78

89
/**
910
* A call to the `Promise` constructor, such as `new Promise((resolve, reject) => { ... })`.
@@ -397,6 +398,17 @@ module PromiseFlow {
397398
value = call.getCallback(0).getExceptionalReturn() and
398399
obj = call
399400
)
401+
or
402+
exists(DataFlow::FunctionNode f | f.getFunction().isAsync() |
403+
// ordinary return
404+
prop = valueProp() and
405+
value = f.getAReturn() and
406+
obj = f.getReturnNode()
407+
or
408+
// exceptional return
409+
prop = errorProp() and
410+
localExceptionStepWithAsyncFlag(value, obj, true)
411+
)
400412
}
401413

402414
/**
@@ -525,30 +537,6 @@ private class PromiseTaintStep extends TaintTracking::LegacyTaintStep {
525537
* Defines flow steps for return on async functions.
526538
*/
527539
private module AsyncReturnSteps {
528-
private predicate valueProp = Promises::valueProp/0;
529-
530-
private predicate errorProp = Promises::errorProp/0;
531-
532-
private import semmle.javascript.dataflow.internal.FlowSteps
533-
534-
/**
535-
* A data-flow step for ordinary and exceptional returns from async functions.
536-
*/
537-
private class AsyncReturn extends LegacyPreCallGraphStep {
538-
override predicate storeStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) {
539-
exists(DataFlow::FunctionNode f | f.getFunction().isAsync() |
540-
// ordinary return
541-
prop = valueProp() and
542-
pred = f.getAReturn() and
543-
succ = f.getReturnNode()
544-
or
545-
// exceptional return
546-
prop = errorProp() and
547-
localExceptionStepWithAsyncFlag(pred, succ, true)
548-
)
549-
}
550-
}
551-
552540
/**
553541
* A data-flow step for ordinary return from an async function in a taint configuration.
554542
*/

javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll

+1-1
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ module TaintTracking {
494494
succ = c and
495495
c =
496496
DataFlow::globalVarRef([
497-
"encodeURI", "decodeURI", "encodeURIComponent", "decodeURIComponent"
497+
"encodeURI", "decodeURI", "encodeURIComponent", "decodeURIComponent", "unescape"
498498
]).getACall() and
499499
pred = c.getArgument(0)
500500
)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import * as t from "testlib";
2+
3+
async function getData1() {
4+
const data = await fetch("https://example.com/content");
5+
return data.json(); /* def=moduleImport("testlib").getMember("exports").getMember("foo").getParameter(0).getReturn().getPromised() */
6+
}
7+
8+
export function use1() {
9+
t.foo(() => getData1());
10+
}
11+
12+
async function getData2() {
13+
const data = await fetch("https://example.com/content");
14+
return data.json(); /* def=moduleImport("testlib").getMember("exports").getMember("foo").getParameter(0).getReturn().getPromised() */
15+
}
16+
17+
export function use2() {
18+
t.foo(getData2);
19+
}
20+
21+
export function use3() {
22+
t.foo(async () => {
23+
const data = await fetch("https://example.com/content");
24+
return data.json(); /* def=moduleImport("testlib").getMember("exports").getMember("foo").getParameter(0).getReturn().getPromised() */
25+
});
26+
}

javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected

+8
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@
223223
| tst.js:477:18:477:40 | locatio ... bstr(1) | tst.js:477:18:477:30 | location.hash | tst.js:477:18:477:40 | locatio ... bstr(1) | Cross-site scripting vulnerability due to $@. | tst.js:477:18:477:30 | location.hash | user-provided value |
224224
| tst.js:484:33:484:63 | decodeU ... n.hash) | tst.js:484:43:484:62 | window.location.hash | tst.js:484:33:484:63 | decodeU ... n.hash) | Cross-site scripting vulnerability due to $@. | tst.js:484:43:484:62 | window.location.hash | user-provided value |
225225
| tst.js:492:18:492:54 | target. ... "), '') | tst.js:491:16:491:39 | documen ... .search | tst.js:492:18:492:54 | target. ... "), '') | Cross-site scripting vulnerability due to $@. | tst.js:491:16:491:39 | documen ... .search | user-provided value |
226+
| tst.js:499:18:499:33 | unescape(source) | tst.js:498:16:498:26 | window.name | tst.js:499:18:499:33 | unescape(source) | Cross-site scripting vulnerability due to $@. | tst.js:498:16:498:26 | window.name | user-provided value |
226227
| typeahead.js:25:18:25:20 | val | typeahead.js:20:22:20:45 | documen ... .search | typeahead.js:25:18:25:20 | val | Cross-site scripting vulnerability due to $@. | typeahead.js:20:22:20:45 | documen ... .search | user-provided value |
227228
| v-html.vue:2:8:2:23 | v-html=tainted | v-html.vue:6:42:6:58 | document.location | v-html.vue:2:8:2:23 | v-html=tainted | Cross-site scripting vulnerability due to $@. | v-html.vue:6:42:6:58 | document.location | user-provided value |
228229
| various-concat-obfuscations.js:4:4:4:31 | "<div>" ... </div>" | various-concat-obfuscations.js:2:16:2:39 | documen ... .search | various-concat-obfuscations.js:4:4:4:31 | "<div>" ... </div>" | Cross-site scripting vulnerability due to $@. | various-concat-obfuscations.js:2:16:2:39 | documen ... .search | user-provided value |
@@ -745,6 +746,9 @@ edges
745746
| tst.js:491:7:491:39 | target | tst.js:492:18:492:23 | target | provenance | |
746747
| tst.js:491:16:491:39 | documen ... .search | tst.js:491:7:491:39 | target | provenance | |
747748
| tst.js:492:18:492:23 | target | tst.js:492:18:492:54 | target. ... "), '') | provenance | |
749+
| tst.js:498:7:498:26 | source | tst.js:499:27:499:32 | source | provenance | |
750+
| tst.js:498:16:498:26 | window.name | tst.js:498:7:498:26 | source | provenance | |
751+
| tst.js:499:27:499:32 | source | tst.js:499:18:499:33 | unescape(source) | provenance | |
748752
| typeahead.js:20:13:20:45 | target | typeahead.js:21:12:21:17 | target | provenance | |
749753
| typeahead.js:20:22:20:45 | documen ... .search | typeahead.js:20:13:20:45 | target | provenance | |
750754
| typeahead.js:21:12:21:17 | target | typeahead.js:24:30:24:32 | val | provenance | |
@@ -1397,6 +1401,10 @@ nodes
13971401
| tst.js:491:16:491:39 | documen ... .search | semmle.label | documen ... .search |
13981402
| tst.js:492:18:492:23 | target | semmle.label | target |
13991403
| tst.js:492:18:492:54 | target. ... "), '') | semmle.label | target. ... "), '') |
1404+
| tst.js:498:7:498:26 | source | semmle.label | source |
1405+
| tst.js:498:16:498:26 | window.name | semmle.label | window.name |
1406+
| tst.js:499:18:499:33 | unescape(source) | semmle.label | unescape(source) |
1407+
| tst.js:499:27:499:32 | source | semmle.label | source |
14001408
| typeahead.js:20:13:20:45 | target | semmle.label | target |
14011409
| typeahead.js:20:22:20:45 | documen ... .search | semmle.label | documen ... .search |
14021410
| typeahead.js:21:12:21:17 | target | semmle.label | target |

javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected

+7
Original file line numberDiff line numberDiff line change
@@ -607,6 +607,10 @@ nodes
607607
| tst.js:491:16:491:39 | documen ... .search | semmle.label | documen ... .search |
608608
| tst.js:492:18:492:23 | target | semmle.label | target |
609609
| tst.js:492:18:492:54 | target. ... "), '') | semmle.label | target. ... "), '') |
610+
| tst.js:498:7:498:26 | source | semmle.label | source |
611+
| tst.js:498:16:498:26 | window.name | semmle.label | window.name |
612+
| tst.js:499:18:499:33 | unescape(source) | semmle.label | unescape(source) |
613+
| tst.js:499:27:499:32 | source | semmle.label | source |
610614
| typeahead.js:9:28:9:30 | loc | semmle.label | loc |
611615
| typeahead.js:10:16:10:18 | loc | semmle.label | loc |
612616
| typeahead.js:20:13:20:45 | target | semmle.label | target |
@@ -1186,6 +1190,9 @@ edges
11861190
| tst.js:491:7:491:39 | target | tst.js:492:18:492:23 | target | provenance | |
11871191
| tst.js:491:16:491:39 | documen ... .search | tst.js:491:7:491:39 | target | provenance | |
11881192
| tst.js:492:18:492:23 | target | tst.js:492:18:492:54 | target. ... "), '') | provenance | |
1193+
| tst.js:498:7:498:26 | source | tst.js:499:27:499:32 | source | provenance | |
1194+
| tst.js:498:16:498:26 | window.name | tst.js:498:7:498:26 | source | provenance | |
1195+
| tst.js:499:27:499:32 | source | tst.js:499:18:499:33 | unescape(source) | provenance | |
11891196
| typeahead.js:9:28:9:30 | loc | typeahead.js:10:16:10:18 | loc | provenance | |
11901197
| typeahead.js:20:13:20:45 | target | typeahead.js:21:12:21:17 | target | provenance | |
11911198
| typeahead.js:20:22:20:45 | documen ... .search | typeahead.js:20:13:20:45 | target | provenance | |

0 commit comments

Comments
 (0)