Skip to content

Commit db277e1

Browse files
GrosQuilduGrosQuildu
committed
more tests
1 parent e861255 commit db277e1

3 files changed

Lines changed: 114 additions & 4 deletions

File tree

‎cpp/src/security/DecOverflowWhenComparing/DecOverflowWhenComparing.ql‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ predicate isDefOf(ControlFlowNode node, Variable var) {
2121
node = var.getAnAccess() and node.(VariableAccess).isLValue()
2222
or
2323
node.(DeclStmt).getADeclaration() = var and exists(var.getInitializer())
24+
or
25+
node.(Assignment).getLValue().(VariableAccess).getTarget() = var
2426
}
2527

2628
/**
@@ -55,9 +57,10 @@ where
5557
cmp.getAnOperand().getAChild*() = varAcc and
5658
cmp.getAnOperand() instanceof Zero and
5759

58-
// only if the variable is used after the comparison
60+
// only if the variable is used (not just overwritten) after the comparison
5961
successorGuarded(cmp, varAccAfterOverflow, var) and
6062
cmp.getAnOperand().getAChild*() != varAccAfterOverflow and
63+
not exists(AssignExpr ae | ae.getLValue() = varAccAfterOverflow) and
6164

6265
// var-- > 0 (0 < var--) then accesses only in false branch
6366
// var-- >= 0 then accesses in all branches

‎cpp/test/query-tests/security/DecOverflowWhenComparing/DecOverflowWhenComparing.c‎

Lines changed: 103 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include "../../../include/libc/unistd.h"
44
#include "../../../include/libc/stdint.h"
55

6+
// BAD
67
// from https://github.com/apple-oss-distributions/Libinfo/blob/9fce29e5c5edc15d3ecea55116ca17d3f6350603/lookup.subproj/mdns_module.c#L1033C1-L1079C2
78
char* _mdns_parse_domain_name(const uint8_t *data, uint32_t datalen)
89
{
@@ -20,18 +21,18 @@ char* _mdns_parse_domain_name(const uint8_t *data, uint32_t datalen)
2021

2122
if (domain == NULL) return NULL;
2223

23-
if (len == 0) break; // DNS root (NUL)
24+
if (len == 0) break;
2425

2526
if (j > 0)
2627
{
2728
domain[j++] = datalen ? '.' : '\0';
2829
}
2930

31+
// bug is in datalen decrementation here
3032
while ((len-- > 0) && (0 != datalen--))
3133
{
3234
if (data[i] == '.')
3335
{
34-
/* special case: escape the '.' with a '\' */
3536
domain = reallocf(domain, ++domainlen);
3637
if (domain == NULL) return NULL;
3738

@@ -47,6 +48,106 @@ char* _mdns_parse_domain_name(const uint8_t *data, uint32_t datalen)
4748
return domain;
4849
}
4950

51+
// BAD: n used after loop without overwrite (overflowed on last iteration)
52+
void simple_use_after_loop(uint32_t n, int *buf) {
53+
while (n-- > 0) {
54+
buf[0] = 1;
55+
}
56+
buf[n] = 0; // BAD: n is UINT32_MAX here
57+
}
58+
59+
// BAD: use in false branch of != 0
60+
void use_after_neq(uint32_t n, int *buf) {
61+
if (n-- != 0) {
62+
buf[0] = 1;
63+
}
64+
buf[n] = 0; // BAD
65+
}
66+
67+
// GOOD: variable overwritten by assignment before use
68+
void overwrite_by_assignment(uint32_t n, int *buf) {
69+
while (n-- > 0) {
70+
buf[0] = 1;
71+
}
72+
n = 42;
73+
buf[n] = 0;
74+
}
75+
76+
// GOOD: signed integer - not unsigned
77+
void signed_decrement(int n, int *buf) {
78+
while (n-- > 0) {
79+
buf[n] = 0;
80+
}
81+
}
82+
83+
// GOOD: variable not used after comparison
84+
void no_use_after(uint32_t n, int *buf) {
85+
while (n-- > 0) {
86+
buf[0] = 1;
87+
}
88+
}
89+
90+
// BAD: reversed comparison form 0 < n--
91+
void reversed_lt(uint32_t n, int *buf) {
92+
while (0 < n--) {
93+
buf[0] = 1;
94+
}
95+
buf[n] = 0; // BAD
96+
}
97+
98+
// BAD: n-- == 0, use in true branch where n just wrapped to UINT32_MAX
99+
void eq_zero(uint32_t n, int *buf) {
100+
if (n-- == 0) {
101+
buf[n] = 0; // BAD
102+
}
103+
}
104+
105+
// BAD: do-while, use after loop
106+
void do_while(uint32_t n, int *buf) {
107+
do {
108+
buf[0] = 1;
109+
} while (n-- > 0);
110+
buf[n] = 0; // BAD
111+
}
112+
113+
// BAD: conditional overwrite (only one branch overwrites)
114+
void conditional_overwrite(uint32_t n, int *buf, int cond) {
115+
if (n-- != 0) {
116+
buf[0] = 1;
117+
}
118+
if (cond) {
119+
n = 42;
120+
}
121+
buf[n] = 0; // BAD: path exists where n is not overwritten
122+
}
123+
124+
// GOOD: use only in true branch of > 0 (n was confirmed > 0)
125+
void use_in_true_branch(uint32_t n, int *buf) {
126+
if (n-- > 0) {
127+
buf[n] = 0;
128+
}
129+
}
130+
131+
// GOOD: prefix decrement (query only checks postfix)
132+
void prefix_decrement(uint32_t n, int *buf) {
133+
while (--n > 0) {
134+
buf[n] = 0;
135+
}
136+
}
137+
138+
// GOOD: overwrite in all branches before use
139+
void overwrite_all_branches(uint32_t n, int *buf, int cond) {
140+
while (n-- > 0) {
141+
buf[0] = 1;
142+
}
143+
if (cond) {
144+
n = 10;
145+
} else {
146+
n = 20;
147+
}
148+
buf[n] = 0;
149+
}
150+
50151
int main() {
51152
const uint16_t datalen = 128;
52153
uint8_t data[datalen] = {};

‎cpp/test/query-tests/security/DecOverflowWhenComparing/DecOverflowWhenComparing.expected‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,7 @@
1-
| DecOverflowWhenComparing.c:30:31:30:39 | ... -- | Unsigned decrementation in comparison ($@) - $@ | DecOverflowWhenComparing.c:30:26:30:39 | ... != ... | ... != ... | DecOverflowWhenComparing.c:15:9:15:15 | datalen | datalen |
1+
| DecOverflowWhenComparing.c:32:31:32:39 | ... -- | Unsigned decrementation in comparison ($@) - $@ | DecOverflowWhenComparing.c:32:26:32:39 | ... != ... | ... != ... | DecOverflowWhenComparing.c:16:9:16:15 | datalen | datalen |
2+
| DecOverflowWhenComparing.c:53:12:53:14 | ... -- | Unsigned decrementation in comparison ($@) - $@ | DecOverflowWhenComparing.c:53:12:53:18 | ... > ... | ... > ... | DecOverflowWhenComparing.c:56:9:56:9 | n | n |
3+
| DecOverflowWhenComparing.c:61:9:61:11 | ... -- | Unsigned decrementation in comparison ($@) - $@ | DecOverflowWhenComparing.c:61:9:61:16 | ... != ... | ... != ... | DecOverflowWhenComparing.c:64:9:64:9 | n | n |
4+
| DecOverflowWhenComparing.c:92:16:92:18 | ... -- | Unsigned decrementation in comparison ($@) - $@ | DecOverflowWhenComparing.c:92:12:92:18 | ... < ... | ... < ... | DecOverflowWhenComparing.c:95:9:95:9 | n | n |
5+
| DecOverflowWhenComparing.c:100:9:100:11 | ... -- | Unsigned decrementation in comparison ($@) - $@ | DecOverflowWhenComparing.c:100:9:100:16 | ... == ... | ... == ... | DecOverflowWhenComparing.c:101:13:101:13 | n | n |
6+
| DecOverflowWhenComparing.c:109:14:109:16 | ... -- | Unsigned decrementation in comparison ($@) - $@ | DecOverflowWhenComparing.c:109:14:109:20 | ... > ... | ... > ... | DecOverflowWhenComparing.c:110:9:110:9 | n | n |
7+
| DecOverflowWhenComparing.c:115:9:115:11 | ... -- | Unsigned decrementation in comparison ($@) - $@ | DecOverflowWhenComparing.c:115:9:115:16 | ... != ... | ... != ... | DecOverflowWhenComparing.c:121:9:121:9 | n | n |

0 commit comments

Comments
 (0)