Skip to content

Commit 4ca0712

Browse files
authored
Merge branch 'main' into redsun82/update-rules-rust
2 parents 2e04d4b + 2e7da72 commit 4ca0712

File tree

260 files changed

+2821
-3300
lines changed

Some content is hidden

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

260 files changed

+2821
-3300
lines changed

config/add-overlay-annotations.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ def annotate_as_appropriate(filename, lines):
199199
# as overlay[local?]. It is not clear that these heuristics are exactly what we want,
200200
# but they seem to work well enough for now (as determined by speed and accuracy numbers).
201201
if (filename.endswith("Test.qll") or
202+
re.search(r"go/ql/lib/semmle/go/security/[^/]+[.]qll$", filename.replace(os.sep, "/")) or
202203
((filename.endswith("Query.qll") or filename.endswith("Config.qll")) and
203204
any("implements DataFlow::ConfigSig" in line for line in lines))):
204205
return None

config/identical-files.json

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -172,10 +172,6 @@
172172
"cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/reachability/PrintDominance.qll",
173173
"cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/reachability/PrintDominance.qll"
174174
],
175-
"C# ControlFlowReachability": [
176-
"csharp/ql/lib/semmle/code/csharp/dataflow/internal/ControlFlowReachability.qll",
177-
"csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/ControlFlowReachability.qll"
178-
],
179175
"C++ ExternalAPIs": [
180176
"cpp/ql/src/Security/CWE/CWE-020/ExternalAPIs.qll",
181177
"cpp/ql/src/Security/CWE/CWE-020/ir/ExternalAPIs.qll"

cpp/ql/lib/semmle/code/cpp/Function.qll

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,12 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
524524
not exists(NewOrNewArrayExpr new | e = new.getAllocatorCall().getArgument(0))
525525
)
526526
}
527+
528+
/**
529+
* Holds if this function has an ambiguous return type, meaning that zero or multiple return
530+
* types for this function are present in the database (this can occur in `build-mode: none`).
531+
*/
532+
predicate hasAmbiguousReturnType() { count(this.getType()) != 1 }
527533
}
528534

529535
pragma[noinline]

cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll

Lines changed: 42 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -6,117 +6,67 @@ private import OverlayXml
66

77
/**
88
* Holds always for the overlay variant and never for the base variant.
9-
* This local predicate is used to define local predicates that behave
10-
* differently for the base and overlay variant.
119
*/
1210
overlay[local]
1311
predicate isOverlay() { databaseMetadata("isOverlay", "true") }
1412

15-
overlay[local]
16-
private string getLocationFilePath(@location_default loc) {
17-
exists(@file file | locations_default(loc, file, _, _, _, _) | files(file, result))
18-
}
19-
20-
/**
21-
* Gets the file path for an element with a single location.
22-
*/
23-
overlay[local]
24-
private string getSingleLocationFilePath(@element e) {
25-
exists(@location_default loc |
26-
var_decls(e, _, _, _, loc)
27-
or
28-
fun_decls(e, _, _, _, loc)
29-
or
30-
type_decls(e, _, loc)
31-
or
32-
namespace_decls(e, _, loc, _)
33-
or
34-
macroinvocations(e, _, loc, _)
35-
or
36-
preprocdirects(e, _, loc)
37-
or
38-
diagnostics(e, _, _, _, _, loc)
39-
or
40-
usings(e, _, loc, _)
41-
or
42-
static_asserts(e, _, _, loc, _)
43-
or
44-
derivations(e, _, _, _, loc)
45-
or
46-
frienddecls(e, _, _, loc)
47-
or
48-
comments(e, _, loc)
49-
or
50-
exprs(e, _, loc)
51-
or
52-
stmts(e, _, loc)
53-
or
54-
initialisers(e, _, _, loc)
55-
or
56-
attributes(e, _, _, _, loc)
57-
or
58-
attribute_args(e, _, _, _, loc)
59-
or
60-
namequalifiers(e, _, _, loc)
61-
or
62-
enumconstants(e, _, _, _, _, loc)
63-
or
64-
type_mentions(e, _, loc, _)
65-
or
66-
lambda_capture(e, _, _, _, _, _, loc)
67-
or
68-
concept_templates(e, _, loc)
69-
|
70-
result = getLocationFilePath(loc)
71-
)
72-
}
73-
7413
/**
75-
* Gets the file path for an element with potentially multiple locations.
14+
* Holds if the TRAP file or tag `t` is reachable from source file `sourceFile`
15+
* in the base (isOverlayVariant=false) or overlay (isOverlayVariant=true) variant.
7616
*/
7717
overlay[local]
78-
private string getMultiLocationFilePath(@element e) {
79-
exists(@location_default loc |
80-
var_decls(_, e, _, _, loc)
81-
or
82-
fun_decls(_, e, _, _, loc)
83-
or
84-
type_decls(_, e, loc)
85-
or
86-
namespace_decls(_, e, loc, _)
87-
|
88-
result = getLocationFilePath(loc)
18+
private predicate locallyReachableTrapOrTag(
19+
boolean isOverlayVariant, string sourceFile, @trap_or_tag t
20+
) {
21+
exists(@source_file sf, @trap trap |
22+
(if isOverlay() then isOverlayVariant = true else isOverlayVariant = false) and
23+
source_file_uses_trap(sf, trap) and
24+
source_file_name(sf, sourceFile) and
25+
(t = trap or trap_uses_tag(trap, t))
8926
)
9027
}
9128

9229
/**
93-
* A local helper predicate that holds in the base variant and never in the
94-
* overlay variant.
95-
*/
96-
overlay[local]
97-
private predicate isBase() { not isOverlay() }
98-
99-
/**
100-
* Holds if `path` was extracted in the overlay database.
30+
* Holds if element `e` is in TRAP file or tag `t`
31+
* in the base (isOverlayVariant=false) or overlay (isOverlayVariant=true) variant.
10132
*/
10233
overlay[local]
103-
private predicate overlayHasFile(string path) {
104-
isOverlay() and
105-
files(_, path) and
106-
path != ""
34+
private predicate locallyInTrapOrTag(boolean isOverlayVariant, @element e, @trap_or_tag t) {
35+
(if isOverlay() then isOverlayVariant = true else isOverlayVariant = false) and
36+
in_trap_or_tag(e, t)
10737
}
10838

10939
/**
11040
* Discards an element from the base variant if:
111-
* - It has a single location in a file extracted in the overlay, or
112-
* - All of its locations are in files extracted in the overlay.
41+
* - We have knowledge about what TRAP file or tag it is in (in the base).
42+
* - It is not in any overlay TRAP file or tag that is reachable from an overlay source file.
43+
* - For every base TRAP file or tag that contains it and is reachable from a base source file,
44+
* either the source file has changed, or the overlay has redefined the TRAP file or tag,
45+
* or the overlay runner has re-extracted the same source file.
11346
*/
11447
overlay[discard_entity]
11548
private predicate discardElement(@element e) {
116-
isBase() and
117-
(
118-
overlayHasFile(getSingleLocationFilePath(e))
119-
or
120-
forex(string path | path = getMultiLocationFilePath(e) | overlayHasFile(path))
49+
// If we don't have any knowledge about what TRAP file something
50+
// is in, then we don't want to discard it, so we only consider
51+
// entities that are known to be in a base TRAP file or tag.
52+
locallyInTrapOrTag(false, e, _) and
53+
// Anything that is reachable from an overlay source file should
54+
// not be discarded.
55+
not exists(@trap_or_tag t | locallyInTrapOrTag(true, e, t) |
56+
locallyReachableTrapOrTag(true, _, t)
57+
) and
58+
// Finally, we have to make sure the base variant does not retain it.
59+
// If it is reachable from a base source file, then that is
60+
// sufficient unless either the base source file has changed (in
61+
// particular, been deleted), or the overlay has redefined the TRAP
62+
// file or tag it is in, or the overlay runner has re-extracted the same
63+
// source file (e.g. because a header it includes has changed).
64+
forall(@trap_or_tag t, string sourceFile |
65+
locallyInTrapOrTag(false, e, t) and
66+
locallyReachableTrapOrTag(false, sourceFile, t)
67+
|
68+
overlayChangedFiles(sourceFile) or
69+
locallyReachableTrapOrTag(true, _, t) or
70+
locallyReachableTrapOrTag(true, sourceFile, _)
12171
)
12272
}

cpp/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,9 @@ where
218218
// only report if we cannot prove that the result of the
219219
// multiplication will be less (resp. greater) than the
220220
// maximum (resp. minimum) number we can compute.
221-
overflows(me, t1)
221+
overflows(me, t1) and
222+
// exclude cases where the expression type may not have been extracted accurately
223+
not me.getParent().(Call).getTarget().hasAmbiguousReturnType()
222224
select me,
223225
"Multiplication result may overflow '" + me.getType().toString() + "' before it is converted to '"
224226
+ me.getFullyConverted().getType().toString() + "'."

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* allows for a cross-site scripting vulnerability.
55
* @kind path-problem
66
* @problem.severity error
7-
* @security-severity 6.1
7+
* @security-severity 7.8
88
* @precision high
99
* @id cpp/cgi-xss
1010
* @tags security
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Fixed an issue with the "Multiplication result converted to larger type" (`cpp/integer-multiplication-cast-to-long`) query causing false positive results in `build-mode: none` databases.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: queryMetadata
3+
---
4+
* The `@security-severity` metadata of `cpp/cgi-xss` has been increased from 6.1 (medium) to 7.8 (high).
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// semmle-extractor-options: --expect_errors
2+
3+
void test_float_double1(float f, double d) {
4+
float r1 = f * f; // GOOD
5+
float r2 = f * d; // GOOD
6+
double r3 = f * f; // BAD
7+
double r4 = f * d; // GOOD
8+
9+
float f1 = fabsf(f * f); // GOOD
10+
float f2 = fabsf(f * d); // GOOD
11+
double f3 = fabs(f * f); // BAD [NOT DETECTED]
12+
double f4 = fabs(f * d); // GOOD
13+
}
14+
15+
double fabs(double f);
16+
float fabsf(float f);
17+
18+
void test_float_double2(float f, double d) {
19+
float r1 = f * f; // GOOD
20+
float r2 = f * d; // GOOD
21+
double r3 = f * f; // BAD
22+
double r4 = f * d; // GOOD
23+
24+
float f1 = fabsf(f * f); // GOOD
25+
float f2 = fabsf(f * d); // GOOD
26+
double f3 = fabs(f * f); // BAD [NOT DETECTED]
27+
double f4 = fabs(f * d); // GOOD
28+
}

cpp/ql/test/query-tests/Likely Bugs/Arithmetic/IntMultToLong/IntMultToLong.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
| Buildless.c:6:17:6:21 | ... * ... | Multiplication result may overflow 'float' before it is converted to 'double'. |
2+
| Buildless.c:21:17:21:21 | ... * ... | Multiplication result may overflow 'float' before it is converted to 'double'. |
13
| IntMultToLong.c:4:10:4:14 | ... * ... | Multiplication result may overflow 'int' before it is converted to 'long long'. |
24
| IntMultToLong.c:7:16:7:20 | ... * ... | Multiplication result may overflow 'int' before it is converted to 'long long'. |
35
| IntMultToLong.c:18:19:18:23 | ... * ... | Multiplication result may overflow 'float' before it is converted to 'double'. |

0 commit comments

Comments
 (0)