Skip to content

Commit d28b90f

Browse files
author
Nikita Kraiouchkine
authored
Merge branch 'main' into OutOfBounds
2 parents 5f8a6f7 + 015fc83 commit d28b90f

File tree

178 files changed

+10498
-820
lines changed

Some content is hidden

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

178 files changed

+10498
-820
lines changed

Diff for: .github/workflows/dispatch-matrix-test-on-comment.yml

+7-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@ name: 🤖 Run Matrix Check (On Comment)
33
on:
44
issue_comment:
55
types: [created]
6+
branches:
7+
- main
8+
- "rc/**"
9+
- next
10+
611

712
jobs:
813
dispatch-matrix-check:
@@ -33,11 +38,12 @@ jobs:
3338
client-payload: '{"pr": "${{ github.event.number }}"}'
3439

3540
- uses: actions/github-script@v6
41+
if: ${{ github.event.issue.pull_request && contains(github.event.comment.body, '/test-matrix') }}
3642
with:
3743
script: |
3844
github.rest.issues.createComment({
3945
issue_number: context.issue.number,
4046
owner: context.repo.owner,
4147
repo: context.repo.repo,
4248
body: '🤖 Beep Boop! Matrix Testing for this PR has been initiated. Please check back later for results. <br><br> :bulb: If you do not hear back from me please check my status! **I will report even if this PR does not contain files eligible for matrix testing.**'
43-
})
49+
})

Diff for: .vscode/tasks.json

+5-2
Original file line numberDiff line numberDiff line change
@@ -203,8 +203,8 @@
203203
"Concurrency1",
204204
"Concurrency2",
205205
"Concurrency3",
206-
"Concurrency4",
207-
"Concurrency5",
206+
"Concurrency4",
207+
"Concurrency5",
208208
"Conditionals",
209209
"Const",
210210
"DeadCode",
@@ -242,6 +242,7 @@
242242
"Macros",
243243
"Memory1",
244244
"Memory2",
245+
"Memory3",
245246
"Misc",
246247
"MoveForward",
247248
"Naming",
@@ -253,9 +254,11 @@
253254
"Pointers1",
254255
"Pointers2",
255256
"Pointers3",
257+
"Representation",
256258
"Scope",
257259
"SideEffects1",
258260
"SideEffects2",
261+
"SideEffects3",
259262
"SmartPointers1",
260263
"SmartPointers2",
261264
"Strings",

Diff for: c/cert/src/qlpack.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: codeql/cert-c-coding-standards
2-
version: 2.15.0-dev
2+
version: 2.16.0-dev
33
description: CERT C 2016
44
suites: codeql-suites
55
license: MIT
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
# ARR32-C: Ensure size arguments for variable length arrays are in a valid range
2+
3+
This query implements the CERT-C rule ARR32-C:
4+
5+
> Ensure size arguments for variable length arrays are in a valid range
6+
7+
8+
## Description
9+
10+
Variable length arrays (VLAs), a conditionally supported language feature, are essentially the same as traditional C arrays except that they are declared with a size that is not a constant integer expression and can be declared only at block scope or function prototype scope and no linkage. When supported, a variable length array can be declared
11+
12+
```cpp
13+
{ /* Block scope */
14+
char vla[size];
15+
}
16+
17+
```
18+
where the integer expression `size` and the declaration of `vla` are both evaluated at runtime. If the size argument supplied to a variable length array is not a positive integer value, the behavior is undefined. (See [undefined behavior 75](https://wiki.sei.cmu.edu/confluence/display/c/CC.+Undefined+Behavior#CC.UndefinedBehavior-ub_75).) Additionally, if the magnitude of the argument is excessive, the program may behave in an unexpected way. An attacker may be able to leverage this behavior to overwrite critical program data \[[Griffiths 2006](https://wiki.sei.cmu.edu/confluence/display/c/AA.+Bibliography#AA.Bibliography-Griffiths06)\]. The programmer must ensure that size arguments to variable length arrays, especially those derived from untrusted data, are in a valid range.
19+
20+
Because variable length arrays are a conditionally supported feature of C11, their use in portable code should be guarded by testing the value of the macro `__STDC_NO_VLA__`. Implementations that do not support variable length arrays indicate it by setting `__STDC_NO_VLA__` to the integer constant 1.
21+
22+
## Noncompliant Code Example
23+
24+
In this noncompliant code example, a variable length array of size `size` is declared. The `size` is declared as `size_t` in compliance with [INT01-C. Use rsize_t or size_t for all integer values representing the size of an object](https://wiki.sei.cmu.edu/confluence/display/c/INT01-C.+Use+rsize_t+or+size_t+for+all+integer+values+representing+the+size+of+an+object).
25+
26+
```cpp
27+
#include <stddef.h>
28+
29+
extern void do_work(int *array, size_t size);
30+
31+
void func(size_t size) {
32+
int vla[size];
33+
do_work(vla, size);
34+
}
35+
36+
```
37+
However, the value of `size` may be zero or excessive, potentially giving rise to a security [vulnerability](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-vulnerability).
38+
39+
## Compliant Solution
40+
41+
This compliant solution ensures the `size` argument used to allocate `vla` is in a valid range (between 1 and a programmer-defined maximum); otherwise, it uses an algorithm that relies on dynamic memory allocation. The solution also avoids unsigned integer wrapping that, given a sufficiently large value of `size`, would cause `malloc` to allocate insufficient storage for the array.
42+
43+
```cpp
44+
#include <stdint.h>
45+
#include <stdlib.h>
46+
47+
enum { MAX_ARRAY = 1024 };
48+
extern void do_work(int *array, size_t size);
49+
50+
void func(size_t size) {
51+
if (0 == size || SIZE_MAX / sizeof(int) < size) {
52+
/* Handle error */
53+
return;
54+
}
55+
if (size < MAX_ARRAY) {
56+
int vla[size];
57+
do_work(vla, size);
58+
} else {
59+
int *array = (int *)malloc(size * sizeof(int));
60+
if (array == NULL) {
61+
/* Handle error */
62+
}
63+
do_work(array, size);
64+
free(array);
65+
}
66+
}
67+
68+
```
69+
70+
## Noncompliant Code Example (sizeof)
71+
72+
The following noncompliant code example defines `A` to be a variable length array and then uses the `sizeof` operator to compute its size at runtime. When the function is called with an argument greater than `SIZE_MAX / (N1 * sizeof (int))`, the runtime `sizeof` expression may wrap around, yielding a result that is smaller than the mathematical product `N1 * n2 * sizeof (int)`. The call to `malloc()`, when successful, will then allocate storage for fewer than `n2` elements of the array, causing one or more of the final `memset()` calls in the `for` loop to write past the end of that storage.
73+
74+
```cpp
75+
#include <stdlib.h>
76+
#include <string.h>
77+
78+
enum { N1 = 4096 };
79+
80+
void *func(size_t n2) {
81+
typedef int A[n2][N1];
82+
83+
A *array = malloc(sizeof(A));
84+
if (!array) {
85+
/* Handle error */
86+
return NULL;
87+
}
88+
89+
for (size_t i = 0; i != n2; ++i) {
90+
memset(array[i], 0, N1 * sizeof(int));
91+
}
92+
93+
return array;
94+
}
95+
96+
```
97+
Furthermore, this code also violates [ARR39-C. Do not add or subtract a scaled integer to a pointer](https://wiki.sei.cmu.edu/confluence/display/c/ARR39-C.+Do+not+add+or+subtract+a+scaled+integer+to+a+pointer), where `array` is a pointer to the two-dimensional array, where it should really be a pointer to the latter dimension instead. This means that the `memset() `call does out-of-bounds writes on all of its invocations except the first.
98+
99+
## Compliant Solution (sizeof)
100+
101+
This compliant solution prevents `sizeof` wrapping by detecting the condition before it occurs and avoiding the subsequent computation when the condition is detected. The code also uses an additional typedef to fix the type of `array` so that `memset()` never writes past the two-dimensional array.
102+
103+
```cpp
104+
#include <stdint.h>
105+
#include <stdlib.h>
106+
#include <string.h>
107+
108+
enum { N1 = 4096 };
109+
110+
void *func(size_t n2) {
111+
if (n2 > SIZE_MAX / (N1 * sizeof(int))) {
112+
/* Prevent sizeof wrapping */
113+
return NULL;
114+
}
115+
116+
typedef int A1[N1];
117+
typedef A1 A[n2];
118+
119+
A1 *array = (A1*) malloc(sizeof(A));
120+
121+
if (!array) {
122+
/* Handle error */
123+
return NULL;
124+
}
125+
126+
for (size_t i = 0; i != n2; ++i) {
127+
memset(array[i], 0, N1 * sizeof(int));
128+
}
129+
return array;
130+
}
131+
132+
```
133+
**Implementation Details**
134+
135+
**Microsoft**
136+
137+
Variable length arrays are not supported by Microsoft compilers.
138+
139+
## Risk Assessment
140+
141+
Failure to properly specify the size of a variable length array may allow arbitrary code execution or result in stack exhaustion.
142+
143+
<table> <tbody> <tr> <th> Rule </th> <th> Severity </th> <th> Likelihood </th> <th> Remediation Cost </th> <th> Priority </th> <th> Level </th> </tr> <tr> <td> ARR32-C </td> <td> High </td> <td> Probable </td> <td> High </td> <td> <strong>P6</strong> </td> <td> <strong>L2</strong> </td> </tr> </tbody> </table>
144+
145+
146+
## Automated Detection
147+
148+
<table> <tbody> <tr> <th> Tool </th> <th> Version </th> <th> Checker </th> <th> Description </th> </tr> <tr> <td> <a> CodeSonar </a> </td> <td> 7.2p0 </td> <td> <strong>ALLOC.SIZE.IOFLOWALLOC.SIZE.MULOFLOWMISC.MEM.SIZE.BAD</strong> </td> <td> Integer Overflow of Allocation Size Multiplication Overflow of Allocation Size Unreasonable Size Argument </td> </tr> <tr> <td> <a> Coverity </a> </td> <td> 2017.07 </td> <td> <strong>REVERSE_NEGATIVE</strong> </td> <td> Fully implemented </td> </tr> <tr> <td> <a> Helix QAC </a> </td> <td> 2022.4 </td> <td> <strong>C1051</strong> </td> <td> </td> </tr> <tr> <td> <a> Klocwork </a> </td> <td> 2022.4 </td> <td> <strong>MISRA.ARRAY.VAR_LENGTH.2012</strong> </td> <td> </td> </tr> <tr> <td> <a> LDRA tool suite </a> </td> <td> 9.7.1 </td> <td> <strong>621 S</strong> </td> <td> Enhanced enforcement </td> </tr> <tr> <td> <a> Parasoft C/C++test </a> </td> <td> 2022.2 </td> <td> <strong>CERT_C-ARR32-a</strong> </td> <td> Ensure the size of the variable length array is in valid range </td> </tr> <tr> <td> <a> PC-lint Plus </a> </td> <td> 1.4 </td> <td> <strong>9035</strong> </td> <td> Assistance provided </td> </tr> <tr> <td> <a> Polyspace Bug Finder </a> </td> <td> R2023a </td> <td> <a> CERT C: Rule ARR32-C </a> </td> <td> Checks for: Memory allocation with tainted sizeemory allocation with tainted size, tainted size of variable length arrayainted size of variable length array. Rule fully covered. </td> </tr> <tr> <td> <a> PRQA QA-C </a> </td> <td> 9.7 </td> <td> <strong>1051</strong> </td> <td> Partially implemented </td> </tr> <tr> <td> <a> Cppcheck </a> </td> <td> 1.66 </td> <td> <strong>negativeArraySize</strong> </td> <td> Context sensitive analysis Will warn only if given size is negative </td> </tr> <tr> <td> <a> TrustInSoft Analyzer </a> </td> <td> 1.38 </td> <td> <strong>alloca_bounds</strong> </td> <td> Exhaustively verified. </td> </tr> </tbody> </table>
149+
150+
151+
## Related Vulnerabilities
152+
153+
Search for [vulnerabilities](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-vulnerability) resulting from the violation of this rule on the [CERT website](https://www.kb.cert.org/vulnotes/bymetric?searchview&query=FIELD+KEYWORDS+contains+ARR32-C).
154+
155+
## Related Guidelines
156+
157+
[Key here](https://wiki.sei.cmu.edu/confluence/display/c/How+this+Coding+Standard+is+Organized#HowthisCodingStandardisOrganized-RelatedGuidelines) (explains table format and definitions)
158+
159+
<table> <tbody> <tr> <th> Taxonomy </th> <th> Taxonomy item </th> <th> Relationship </th> </tr> <tr> <td> <a> CERT C Secure Coding Standard </a> </td> <td> <a> INT01-C. Use rsize_t or size_t for all integer values representing the size of an object </a> </td> <td> Prior to 2018-01-12: CERT: Unspecified Relationship </td> </tr> <tr> <td> <a> ISO/IEC TR 24772:2013 </a> </td> <td> Unchecked Array Indexing \[XYZ\] </td> <td> Prior to 2018-01-12: CERT: Unspecified Relationship </td> </tr> <tr> <td> <a> ISO/IEC TS 17961:2013 </a> </td> <td> Tainted, potentially mutilated, or out-of-domain integer values are used in a restricted sink \[taintsink\] </td> <td> Prior to 2018-01-12: CERT: Unspecified Relationship </td> </tr> <tr> <td> <a> CWE 2.11 </a> </td> <td> <a> CWE-758 </a> </td> <td> 2017-06-29: CERT: Rule subset of CWE </td> </tr> </tbody> </table>
160+
161+
162+
## CERT-CWE Mapping Notes
163+
164+
[Key here](https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152408#HowthisCodingStandardisOrganized-CERT-CWEMappingNotes) for mapping notes
165+
166+
**CWE-129 and ARR32-C**
167+
168+
Intersection( CWE-188, EXP39-C) = Ø
169+
170+
ARR32-C addresses specifying the size of a variable-length array (VLA). CWE-129 addresses invalid array indices, not array sizes.
171+
172+
**CWE-758 and ARR32-C**
173+
174+
Independent( INT34-C, INT36-C, MSC37-C, FLP32-C, EXP33-C, EXP30-C, ERR34-C, ARR32-C)
175+
176+
CWE-758 = Union( ARR32-C, list) where list =
177+
178+
* Undefined behavior that results from anything other than too large a VLA dimension.
179+
**CWE-119 and ARR32-C**
180+
* Intersection( CWE-119, ARR32-C) = Ø
181+
* ARR32-C is not about providing a valid buffer but reading/writing outside it. It is about providing an invalid buffer, or one that exhausts the stack.
182+
183+
## Bibliography
184+
185+
<table> <tbody> <tr> <td> \[ <a> Griffiths 2006 </a> \] </td> </tr> </tbody> </table>
186+
187+
188+
## Implementation notes
189+
190+
None
191+
192+
## References
193+
194+
* CERT-C: [ARR32-C: Ensure size arguments for variable length arrays are in a valid range](https://wiki.sei.cmu.edu/confluence/display/c)

0 commit comments

Comments
 (0)