Skip to content

Commit 46069af

Browse files
committed
C++: Update documentation for cpp/uncontrolled-allocation-size to clarify its scope
1 parent fbcb449 commit 46069af

File tree

4 files changed

+21
-19
lines changed

4 files changed

+21
-19
lines changed
Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
int factor = atoi(getenv("BRANCHING_FACTOR"));
22

3-
// GOOD: Prevent overflow by checking the input
4-
if (factor < 0 || factor > 1000) {
5-
log("Factor out of range (%d)\n", factor);
6-
return -1;
7-
}
8-
9-
// This line can allocate too little memory if factor
10-
// is very large.
3+
// BAD: This can allocate too little memory if factor is very large due to overflow.
114
char **root_node = (char **) malloc(factor * sizeof(char *));
5+
6+
// GOOD: Prevent overflow and unbounded allocation size by checking the input.
7+
if (factor > 0 && factor <= 1000) {
8+
char **root_node = (char **) malloc(factor * sizeof(char *));
9+
}

cpp/ql/src/Security/CWE/CWE-190/TaintedAllocationSize.qhelp

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,16 @@
33
"qhelp.dtd">
44
<qhelp>
55
<overview>
6-
<p>This code calculates an allocation size by multiplying a user input
7-
by a <code>sizeof</code> expression. Since the user input has no
8-
apparent guard on its magnitude, this multiplication can
9-
overflow. When an integer multiply overflows in C, the result can wrap
10-
around and be much smaller than intended. A later attempt to put data
11-
into the allocated buffer can then overflow.</p>
6+
7+
<p>This code allocates memory using a size value based on user input
8+
with no apparent bound on its magnitude being established. This allows
9+
for arbitrary amounts of memory being allocated.</p>
10+
11+
<p>If the allocation size is calculated by multiplying user input by a
12+
<code>sizeof</code> expression the multiplication can overflow. When
13+
an integer multiplication overflows in C, the result wraps around and
14+
can be much smaller than intended. A later attempt to write data into
15+
the allocated memory can then be out-of-bounds.</p>
1216

1317
</overview>
1418
<recommendation>

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
2-
* @name Overflow in uncontrolled allocation size
3-
* @description Allocating memory with a size controlled by an external
4-
* user can result in integer overflow.
2+
* @name Uncontrolled allocation size
3+
* @description Allocating memory with a size controlled by an external user can result in
4+
* arbitrary amounts of memory being allocated.
55
* @kind path-problem
66
* @problem.severity error
77
* @security-severity 8.1

cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/TaintedAllocationSize/test.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@ int main(int argc, char **argv) {
4040
int tainted = atoi(argv[1]);
4141

4242
MyStruct *arr1 = (MyStruct *)malloc(sizeof(MyStruct)); // GOOD
43-
MyStruct *arr2 = (MyStruct *)malloc(tainted); // DUBIOUS (not multiplied by anything)
43+
MyStruct *arr2 = (MyStruct *)malloc(tainted); // BAD
4444
MyStruct *arr3 = (MyStruct *)malloc(tainted * sizeof(MyStruct)); // BAD
4545
MyStruct *arr4 = (MyStruct *)malloc(getTainted() * sizeof(MyStruct)); // BAD [NOT DETECTED]
46-
MyStruct *arr5 = (MyStruct *)malloc(sizeof(MyStruct) + tainted); // DUBIOUS (not multiplied by anything)
46+
MyStruct *arr5 = (MyStruct *)malloc(sizeof(MyStruct) + tainted); // BAD
4747

4848
int size = tainted * 8;
4949
char *chars1 = (char *)malloc(size); // BAD

0 commit comments

Comments
 (0)