Skip to content

Commit 314a849

Browse files
authored
Merge branch 'main' into alan/crypto_doc_update
2 parents 6209ec9 + 391bf34 commit 314a849

19 files changed

Lines changed: 1844 additions & 9 deletions

README.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ codeql database analyze database.db --format=sarif-latest --output=./tob.sarif -
3030
| Name | Description | Severity | Precision |
3131
| --- | ----------- | :----: | :--------: |
3232
|[BN_CTX_free called before BN_CTX_end](./cpp/src/docs/crypto/BnCtxFreeBeforeEnd.md)|Detects BN_CTX_free called before BN_CTX_end, which violates the required lifecycle|error|medium|
33+
|[Unbalanced BN_CTX_start and BN_CTX_end pair](./cpp/src/docs/crypto/UnbalancedBnCtx.md)|Detects if one call in the BN_CTX_start/BN_CTX_end pair is missing|warning|medium|
3334
|[Crypto variable initialized using static key](./cpp/src/docs/crypto/StaticKeyFlow.md)|Finds crypto variables initialized using static keys|error|high|
3435
|[Crypto variable initialized using static password](./cpp/src/docs/crypto/StaticPasswordFlow.md)|Finds crypto variables initialized using static passwords|error|high|
3536
|[Crypto variable initialized using weak randomness](./cpp/src/docs/crypto/WeakRandomnessTaint.md)|Finds crypto variables initialized using weak randomness|error|high|
@@ -40,17 +41,18 @@ codeql database analyze database.db --format=sarif-latest --output=./tob.sarif -
4041
|[Missing error handling](./cpp/src/docs/crypto/MissingErrorHandling.md)|Checks if returned error codes are properly checked|warning|high|
4142
|[Missing zeroization of potentially sensitive random BIGNUM](./cpp/src/docs/crypto/MissingZeroization.md)|Determines if random bignums are properly zeroized|warning|medium|
4243
|[Random buffer too small](./cpp/src/docs/crypto/RandomBufferTooSmall.md)|Finds buffer overflows in calls to CSPRNGs|warning|high|
43-
|[Unbalanced BN_CTX_start and BN_CTX_end pair](./cpp/src/docs/crypto/UnbalancedBnCtx.md)|Detects if one call in the BN_CTX_start/BN_CTX_end pair is missing|warning|medium|
4444
|[Use of legacy cryptographic algorithm](./cpp/src/docs/crypto/UseOfLegacyAlgorithm.md)|Detects potential instantiations of legacy cryptographic algorithms|warning|medium|
4545

4646
#### Security
4747

4848
| Name | Description | Severity | Precision |
4949
| --- | ----------- | :----: | :--------: |
5050
|[Async unsafe signal handler](./cpp/src/docs/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.md)|Async unsafe signal handler (like the one used in CVE-2024-6387)|warning|high|
51+
|[Decrementation overflow when comparing](./cpp/src/docs/security/DecOverflowWhenComparing/DecOverflowWhenComparing.md)|This query finds unsigned integer overflows resulting from unchecked decrementation during comparison.|error|high|
52+
|[Find all problematic implicit casts](./cpp/src/docs/security/UnsafeImplicitConversions/UnsafeImplicitConversions.md)|Find all implicit casts that may be problematic. That is, casts that may result in unexpected truncation, reinterpretation or widening of values.|error|high|
53+
|[Inconsistent handling of return values from a specific function](./cpp/src/docs/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.md)|Detects functions whose return values are compared inconsistently across call sites, which may indicate bugs|warning|medium|
5154
|[Invalid string size passed to string manipulation function](./cpp/src/docs/security/CStrnFinder/CStrnFinder.md)|Finds calls to functions that take as input a string and its size as separate arguments (e.g., `strncmp`, `strncat`, ...) and the size argument is wrong|error|low|
5255
|[Missing null terminator](./cpp/src/docs/security/NoNullTerminator/NoNullTerminator.md)|This query finds incorrectly initialized strings that are passed to functions expecting null-byte-terminated strings|error|high|
53-
|[Unsafe implicit integer conversion](./cpp/src/docs/security/UnsafeImplicitConversions/UnsafeImplicitConversions.md)|Finds implicit integer casts that may overflow or be truncated, with false positive reduction via Value Range Analysis|warning|low|
5456

5557
### Go
5658

@@ -65,7 +67,7 @@ codeql database analyze database.db --format=sarif-latest --output=./tob.sarif -
6567
| Name | Description | Severity | Precision |
6668
| --- | ----------- | :----: | :--------: |
6769
|[Invalid file permission parameter](./go/src/docs/security/FilePermsFlaws/FilePermsFlaws.md)|Finds non-octal (e.g., `755` vs `0o755`) and unsupported (e.g., `04666`) literals used as a filesystem permission parameter (`FileMode`)|error|medium|
68-
|[Missing MinVersion in tls.Config](./go/src/docs/security/MissingMinVersionTLS/MissingMinVersionTLS.md)|This rule finds cases when you do not set the `tls.Config.MinVersion` explicitly for servers. By default version 1.0 is used, which is considered insecure. This rule does not mark explicitly set insecure versions|error|medium|
70+
|[Missing MinVersion in tls.Config](./go/src/docs/security/MissingMinVersionTLS/MissingMinVersionTLS.md)|Finds uses of tls.Config where MinVersion is not set and the project's minimum Go version (from go.mod) indicates insecure defaults: Go < 1.18 for clients or Go < 1.22 for servers. Does not mark explicitly set versions (including explicitly insecure ones).|error|medium|
6971
|[Trim functions misuse](./go/src/docs/security/TrimMisuse/TrimMisuse.md)|Finds calls to `string.{Trim,TrimLeft,TrimRight}` with the 2nd argument not being a cutset but a continuous substring to be trimmed|error|low|
7072

7173
### Java-kotlin
@@ -74,7 +76,7 @@ codeql database analyze database.db --format=sarif-latest --output=./tob.sarif -
7476

7577
| Name | Description | Severity | Precision |
7678
| --- | ----------- | :----: | :--------: |
77-
|[Recursive functions](./java-kotlin/src/docs/security/Recursion/Recursion.md)|Detects recursive calls|warning|low|
79+
|[Recursive functions](./java-kotlin/src/docs/security/Recursion/Recursion.md)|Detects possibly unbounded recursive calls|warning|low|
7880

7981
## Query suites
8082

cpp/src/crypto/UseOfLegacyAlgorithm.ql

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,10 @@ where
3636
* descend
3737
* destroy
3838
*/
39-
40-
cipherName = "DES" and
41-
functionName.regexpMatch(".*(?<!no|mo|co)des(?!cri(be|ption|ptor)|ign|cend|troy).*")
39+
(
40+
cipherName = "DES" and
41+
functionName.regexpMatch(".*(?<!no|mo|co)des(?!cri(be|ption|ptor)|ign|cend|troy).*")
42+
)
4243
)
4344
select call.getLocation(),
4445
"Potential use of legacy cryptographic algorithm " + cipherName + " detected in function name " +
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
# Decrementation overflow when comparing
2+
Finds unsigned integer overflow issues with the following heuristic: \* variable is compared to 0 and decremented \* variable is used after the comparison and decrementation In such cases it is likely that the decrementation was not expected.
3+
4+
You can read about a real-world vulnerability here: https://github.com/trailofbits/exploits/tree/main/obts-2025-macos-lpe
5+
6+
7+
## Recommendation
8+
Move the decrementation outside of comparison and/or add explicit checks for overflows.
9+
10+
11+
## Example
12+
13+
```c
14+
#include <stdio.h>
15+
#include <stdint.h>
16+
#include <stdlib.h>
17+
#include <string.h>
18+
19+
// from https://github.com/apple-oss-distributions/Libinfo/blob/9fce29e5c5edc15d3ecea55116ca17d3f6350603/lookup.subproj/mdns_module.c#L1033C1-L1079C2
20+
char* _mdns_parse_domain_name(const uint8_t *data, uint32_t datalen)
21+
{
22+
int i = 0, j = 0;
23+
uint32_t domainlen = 0;
24+
char *domain = NULL;
25+
26+
if ((data == NULL) || (datalen == 0)) return NULL;
27+
28+
while (datalen-- > 0)
29+
{
30+
uint32_t len = data[i++];
31+
domainlen += (len + 1);
32+
domain = reallocf(domain, domainlen);
33+
34+
if (domain == NULL) return NULL;
35+
36+
if (len == 0) break; // DNS root (NUL)
37+
38+
if (j > 0)
39+
{
40+
domain[j++] = datalen ? '.' : '\0';
41+
}
42+
43+
while ((len-- > 0) && (0 != datalen--))
44+
{
45+
if (data[i] == '.')
46+
{
47+
/* special case: escape the '.' with a '\' */
48+
domain = reallocf(domain, ++domainlen);
49+
if (domain == NULL) return NULL;
50+
51+
domain[j++] = '\\';
52+
}
53+
54+
domain[j++] = data[i++];
55+
}
56+
}
57+
58+
domain[j] = '\0';
59+
60+
return domain;
61+
}
62+
63+
int main() {
64+
const uint16_t datalen = 128;
65+
uint8_t data[datalen] = {};
66+
memcpy(data, "\x04quildu\x03xyz\x00", 11);
67+
_mdns_parse_domain_name(data, datalen);
68+
}
69+
70+
```
71+
The `datalen` variable may overflow to UINT_MAX given a specific input.
72+
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
# Inconsistent handling of return values from a specific function
2+
When a function's return value is checked in `if` statements across multiple call sites, the comparisons typically fall into a consistent pattern (e.g., compared against a numeric literal, `NULL`, or `sizeof`). If a small number of call sites compare the return value in a different way than the majority, these inconsistent comparisons may indicate a bug.
3+
4+
The query categorizes each comparison into one of the following categories:
5+
6+
* Numeric literal (e.g., `ret != -1`)
7+
* Boolean (e.g., `ret == true`)
8+
* Null pointer (e.g., `ret != NULL`)
9+
* Pointer
10+
* `sizeof` expression (e.g., `ret > sizeof(buf)`)
11+
* Another function's return value (e.g., `ret != other_func()`)
12+
* Passed as argument to another function (e.g., `if (check(ret))`)
13+
* Arithmetic expression
14+
15+
When at least 75% of a function's return value comparisons fall into one category, the remaining comparisons in a different category are flagged as potentially incorrect.
16+
17+
18+
## Recommendation
19+
Review each flagged call site and verify that the comparison matches the function's return value semantics. If the function returns an error code or count, all call sites should compare it consistently. Fix any comparisons that use the wrong type of operand (e.g., comparing an integer return value against `sizeof` when all other sites compare against a numeric literal).
20+
21+
22+
## Example
23+
24+
```c
25+
struct header {
26+
int type;
27+
int length;
28+
};
29+
30+
// Returns number of items processed, or -1 on error
31+
int process_items(int *items, int count) {
32+
int processed = 0;
33+
for (int i = 0; i < count; i++) {
34+
if (items[i] < 0)
35+
return -1;
36+
processed++;
37+
}
38+
return processed;
39+
}
40+
41+
void example() {
42+
int items[10];
43+
int result;
44+
45+
// Typical: return value compared with a numeric literal
46+
result = process_items(items, 10);
47+
if (result > 0) { /* success */ }
48+
49+
result = process_items(items, 5);
50+
if (result != -1) { /* no error */ }
51+
52+
result = process_items(items, 3);
53+
if (result == 0) { /* empty */ }
54+
55+
result = process_items(items, 8);
56+
if (result >= 1) { /* at least one */ }
57+
58+
// BAD: comparing with sizeof instead of a numeric literal.
59+
// This is inconsistent with all other call sites and likely a bug.
60+
result = process_items(items, 7);
61+
if (result > sizeof(struct header)) { /* wrong comparison */ }
62+
}
63+
```
64+
In this example, `process_items` returns the number of items processed or `-1` on error. Most call sites correctly compare the return value with a numeric literal. However, one call site mistakenly compares it with `sizeof(struct header)`, which is inconsistent with how the return value is used everywhere else.
65+
66+
67+
## References
68+
* [CWE-252: Unchecked Return Value](https://cwe.mitre.org/data/definitions/252.html)
69+
* [CWE-253: Incorrect Check of Function Return Value](https://cwe.mitre.org/data/definitions/253.html)
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
#include <stdio.h>
2+
#include <stdint.h>
3+
#include <stdlib.h>
4+
#include <string.h>
5+
6+
// from https://github.com/apple-oss-distributions/Libinfo/blob/9fce29e5c5edc15d3ecea55116ca17d3f6350603/lookup.subproj/mdns_module.c#L1033C1-L1079C2
7+
char* _mdns_parse_domain_name(const uint8_t *data, uint32_t datalen)
8+
{
9+
int i = 0, j = 0;
10+
uint32_t domainlen = 0;
11+
char *domain = NULL;
12+
13+
if ((data == NULL) || (datalen == 0)) return NULL;
14+
15+
while (datalen-- > 0)
16+
{
17+
uint32_t len = data[i++];
18+
domainlen += (len + 1);
19+
domain = reallocf(domain, domainlen);
20+
21+
if (domain == NULL) return NULL;
22+
23+
if (len == 0) break; // DNS root (NUL)
24+
25+
if (j > 0)
26+
{
27+
domain[j++] = datalen ? '.' : '\0';
28+
}
29+
30+
while ((len-- > 0) && (0 != datalen--))
31+
{
32+
if (data[i] == '.')
33+
{
34+
/* special case: escape the '.' with a '\' */
35+
domain = reallocf(domain, ++domainlen);
36+
if (domain == NULL) return NULL;
37+
38+
domain[j++] = '\\';
39+
}
40+
41+
domain[j++] = data[i++];
42+
}
43+
}
44+
45+
domain[j] = '\0';
46+
47+
return domain;
48+
}
49+
50+
int main() {
51+
const uint16_t datalen = 128;
52+
uint8_t data[datalen] = {};
53+
memcpy(data, "\x04quildu\x03xyz\x00", 11);
54+
_mdns_parse_domain_name(data, datalen);
55+
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>
7+
Finds unsigned integer overflow issues with the following heuristic:
8+
* variable is compared to 0 and decremented
9+
* variable is used after the comparison and decrementation
10+
11+
In such cases it is likely that the decrementation was not expected.
12+
</p>
13+
14+
<p>
15+
You can read about a real-world vulnerability here: https://github.com/trailofbits/exploits/tree/main/obts-2025-macos-lpe
16+
</p>
17+
</overview>
18+
19+
<recommendation>
20+
<p>
21+
Move the decrementation outside of comparison and/or add explicit checks for overflows.
22+
</p>
23+
</recommendation>
24+
25+
<example>
26+
<sample src="DecOverflowWhenComparing.c" />
27+
28+
<p>
29+
The <code>datalen</code> variable may overflow to UINT_MAX given a specific input.
30+
</p>
31+
</example>
32+
33+
</qhelp>
34+

0 commit comments

Comments
 (0)