Skip to content

Commit 371c778

Browse files
author
Nikita Kraiouchkine
committed
InvalidMemory2: Implement ARR37-C query
1 parent 11a6941 commit 371c778

5 files changed

+353
-0
lines changed

‎c/cert/src/rules/ARR37-C/DoNotUsePointerArithmeticOnNonArrayObjectPointers.md

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
# ARR37-C: Do not add or subtract an integer to a pointer to a non-array object
2+
3+
This query implements the CERT-C rule ARR37-C:
4+
5+
> Do not add or subtract an integer to a pointer to a non-array object
6+
7+
8+
## Description
9+
10+
Pointer arithmetic must be performed only on pointers that reference elements of array objects.
11+
12+
The C Standard, 6.5.6 \[[ISO/IEC 9899:2011](https://wiki.sei.cmu.edu/confluence/display/c/AA.+Bibliography#AA.Bibliography-ISO-IEC9899-2011)\], states the following about pointer arithmetic:
13+
14+
> When an expression that has integer type is added to or subtracted from a pointer, the result has the type of the pointer operand. If the pointer operand points to an element of an array object, and the array is large enough, the result points to an element offset from the original element such that the difference of the subscripts of the resulting and original array elements equals the integer expression.
15+
16+
17+
## Noncompliant Code Example
18+
19+
This noncompliant code example attempts to access structure members using pointer arithmetic. This practice is dangerous because structure members are not guaranteed to be contiguous.
20+
21+
```cpp
22+
struct numbers {
23+
short num_a, num_b, num_c;
24+
};
25+
26+
int sum_numbers(const struct numbers *numb){
27+
int total = 0;
28+
const short *numb_ptr;
29+
30+
for (numb_ptr = &numb->num_a;
31+
numb_ptr <= &numb->num_c;
32+
numb_ptr++) {
33+
total += *(numb_ptr);
34+
}
35+
36+
return total;
37+
}
38+
39+
int main(void) {
40+
struct numbers my_numbers = { 1, 2, 3 };
41+
sum_numbers(&my_numbers);
42+
return 0;
43+
}
44+
45+
```
46+
47+
## Compliant Solution
48+
49+
It is possible to use the `->` operator to dereference each structure member:
50+
51+
```cpp
52+
total = numb->num_a + numb->num_b + numb->num_c;
53+
54+
```
55+
However, this solution results in code that is hard to write and hard to maintain (especially if there are many more structure members), which is exactly what the author of the noncompliant code example was likely trying to avoid.
56+
57+
## Compliant Solution
58+
59+
A better solution is to define the structure to contain an array member to store the numbers in an array rather than a structure, as in this compliant solution:
60+
61+
```cpp
62+
#include <stddef.h>
63+
64+
struct numbers {
65+
short a[3];
66+
};
67+
68+
int sum_numbers(const short *numb, size_t dim) {
69+
int total = 0;
70+
for (size_t i = 0; i < dim; ++i) {
71+
total += numb[i];
72+
}
73+
74+
return total;
75+
}
76+
77+
int main(void) {
78+
struct numbers my_numbers = { .a[0]= 1, .a[1]= 2, .a[2]= 3};
79+
sum_numbers(
80+
my_numbers.a,
81+
sizeof(my_numbers.a)/sizeof(my_numbers.a[0])
82+
);
83+
return 0;
84+
}
85+
86+
```
87+
Array elements are guaranteed to be contiguous in memory, so this solution is completely portable.
88+
89+
## Exceptions
90+
91+
** ARR37-C-EX1:** Any non-array object in memory can be considered an array consisting of one element. Adding one to a pointer for such an object yields a pointer one element past the array, and subtracting one from that pointer yields the original pointer. This allows for code such as the following:
92+
93+
```cpp
94+
#include <stdlib.h>
95+
#include <string.h>
96+
97+
struct s {
98+
char *c_str;
99+
/* Other members */
100+
};
101+
102+
struct s *create_s(const char *c_str) {
103+
struct s *ret;
104+
size_t len = strlen(c_str) + 1;
105+
106+
ret = (struct s *)malloc(sizeof(struct s) + len);
107+
if (ret != NULL) {
108+
ret->c_str = (char *)(ret + 1);
109+
memcpy(ret + 1, c_str, len);
110+
}
111+
return ret;
112+
}
113+
```
114+
A more general and safer solution to this problem is to use a flexible array member that guarantees the array that follows the structure is properly aligned by inserting padding, if necessary, between it and the member that immediately precedes it.
115+
116+
## Risk Assessment
117+
118+
<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> ARR37-C </td> <td> Medium </td> <td> Probable </td> <td> Medium </td> <td> <strong>P8</strong> </td> <td> <strong>L2</strong> </td> </tr> </tbody> </table>
119+
120+
121+
## Automated Detection
122+
123+
<table> <tbody> <tr> <th> Tool </th> <th> Version </th> <th> Checker </th> <th> Description </th> </tr> <tr> <td> <a> Astrée </a> </td> <td> 22.04 </td> <td> </td> <td> Supported indirectly via MISRA C:2004 Rule 17.4. </td> </tr> <tr> <td> <a> Axivion Bauhaus Suite </a> </td> <td> 7.2.0 </td> <td> <strong>CertC-ARR37</strong> </td> <td> Fully implemented </td> </tr> <tr> <td> <a> CodeSonar </a> </td> <td> 7.2p0 </td> <td> <strong>LANG.MEM.BO</strong> <strong>LANG.MEM.BU</strong> <strong>LANG.STRUCT.PARITH</strong> <strong>LANG.STRUCT.PBB</strong> <strong>LANG.STRUCT.PPE</strong> <strong>LANG.MEM.TBA</strong> <strong>LANG.MEM.TO</strong> <strong>LANG.MEM.TU</strong> </td> <td> Buffer Overrun Buffer Underrun Pointer Arithmetic Pointer Before Beginning of Object Pointer Past End of Object Tainted Buffer Access Type Overrun Type Underrun </td> </tr> <tr> <td> <a> Compass/ROSE </a> </td> <td> </td> <td> </td> <td> </td> </tr> <tr> <td> <a> Coverity </a> </td> <td> 2017.07 </td> <td> <strong>ARRAY_VS_SINGLETON</strong> </td> <td> Implemented </td> </tr> <tr> <td> <a> Helix QAC </a> </td> <td> 2022.4 </td> <td> <strong>DF2930, DF2931, DF2932, DF2933</strong> <strong>C++3705, C++3706, C++3707</strong> </td> <td> </td> </tr> <tr> <td> <a> Klocwork </a> </td> <td> 2022.4 </td> <td> <strong>MISRA.PTR.ARITH.2012</strong> </td> <td> </td> </tr> <tr> <td> <a> LDRA tool suite </a> </td> <td> 9.7.1 </td> <td> <strong>567 S</strong> </td> <td> Partially implemented </td> </tr> <tr> <td> <a> Parasoft C/C++test </a> </td> <td> 2022.2 </td> <td> <strong>CERT_C-ARR37-a</strong> </td> <td> Pointer arithmetic shall not be applied to pointers that address variables of non-array type </td> </tr> <tr> <td> <a> PC-lint Plus </a> </td> <td> 1.4 </td> <td> <strong>2662</strong> </td> <td> Partially supported </td> </tr> <tr> <td> <a> Polyspace Bug Finder </a> </td> <td> R2023a </td> <td> <a> CERT C: Rule ARR37-C </a> </td> <td> Checks for invalid assumptions about memory organization (rule partially covered) </td> </tr> <tr> <td> <a> PRQA QA-C </a> </td> <td> 9.7 </td> <td> <strong>2930, 2931, 2932, 2933</strong> </td> <td> </td> </tr> <tr> <td> <a> PRQA QA-C++ </a> </td> <td> 4.4 </td> <td> <strong>2930, 2931, 2932, 2933,</strong> <strong> 3705, 3706, 3707</strong> </td> <td> </td> </tr> <tr> <td> <a> RuleChecker </a> </td> <td> 22.04 </td> <td> </td> <td> Supported indirectly via MISRA C:2004 Rule 17.4. </td> </tr> </tbody> </table>
124+
125+
126+
## Related Vulnerabilities
127+
128+
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+ARR37-C).
129+
130+
## Related Guidelines
131+
132+
[Key here](https://wiki.sei.cmu.edu/confluence/display/c/How+this+Coding+Standard+is+Organized#HowthisCodingStandardisOrganized-RelatedGuidelines) (explains table format and definitions)
133+
134+
<table> <tbody> <tr> <th> Taxonomy </th> <th> Taxonomy item </th> <th> Relationship </th> </tr> </tbody> </table>
135+
136+
137+
## Bibliography
138+
139+
<table> <tbody> <tr> <td> \[ <a> Banahan 2003 </a> \] </td> <td> <a> Section 5.3, "Pointers" </a> <a> Section 5.7, "Expressions Involving Pointers" </a> </td> </tr> <tr> <td> \[ <a> ISO/IEC 9899:2011 </a> \] </td> <td> 6.5.6, "Additive Operators" </td> </tr> <tr> <td> \[ <a> VU\#162289 </a> \] </td> <td> </td> </tr> </tbody> </table>
140+
141+
142+
## Implementation notes
143+
144+
None
145+
146+
## References
147+
148+
* CERT-C: [ARR37-C: Do not add or subtract an integer to a pointer to a non-array object](https://wiki.sei.cmu.edu/confluence/display/c)

‎c/cert/src/rules/ARR37-C/DoNotUsePointerArithmeticOnNonArrayObjectPointers.ql

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/**
2+
* @id c/cert/do-not-use-pointer-arithmetic-on-non-array-object-pointers
3+
* @name ARR37-C: Do not add or subtract an integer to a pointer to a non-array object
4+
* @description A pair of elements that are not elements in the same array are not guaranteed to be
5+
* contiguous in memory and therefore should not be addressed using pointer arithmetic.
6+
* @kind path-problem
7+
* @precision high
8+
* @problem.severity error
9+
* @tags external/cert/id/arr37-c
10+
* correctness
11+
* external/cert/obligation/rule
12+
*/
13+
14+
import cpp
15+
import codingstandards.c.cert
16+
import semmle.code.cpp.dataflow.DataFlow
17+
import DataFlow::PathGraph
18+
19+
/**
20+
* A data-flow configuration that tracks flow from an `AddressOfExpr` of a variable
21+
* of `PointerType` that is not also an `ArrayType` to a `PointerArithmeticOrArrayExpr`
22+
*/
23+
class NonArrayPointerToArrayIndexingExprConfig extends DataFlow::Configuration {
24+
NonArrayPointerToArrayIndexingExprConfig() { this = "ArrayToArrayIndexConfig" }
25+
26+
override predicate isSource(DataFlow::Node source) {
27+
exists(AddressOfExpr ao, Type t |
28+
source.asExpr() = ao and
29+
not ao.getOperand() instanceof ArrayExpr and
30+
not ao.getOperand() instanceof PointerDereferenceExpr and
31+
t = ao.getOperand().getType() and
32+
not t instanceof PointerType and
33+
not t instanceof ArrayType and
34+
not t.(PointerType).getBaseType() instanceof ArrayType
35+
)
36+
}
37+
38+
override predicate isSink(DataFlow::Node sink) {
39+
exists(PointerArithmeticOrArrayExpr ae |
40+
sink.asExpr() = ae.getPointerOperand() and
41+
not sink.asExpr() instanceof Literal and
42+
not ae.isNonPointerOperandZero()
43+
)
44+
}
45+
46+
override predicate isBarrierOut(DataFlow::Node node) {
47+
// the default interprocedural data-flow model flows through any field or array assignment
48+
// expressions to the qualifier (array base, pointer dereferenced, or qualifier) instead of the
49+
// individual element or field that the assignment modifies. this default behaviour causes
50+
// false positives for future accesses of any element of that object, so we remove the edges
51+
// between those assignments from the graph with `isBarrierOut`.
52+
exists(AssignExpr a |
53+
node.asExpr() = a.getRValue() and
54+
(
55+
a.getLValue() instanceof ArrayExpr or
56+
a.getLValue() instanceof PointerDereferenceExpr or
57+
a.getLValue() instanceof FieldAccess
58+
)
59+
)
60+
or
61+
// ignore AddressOfExpr output e.g. call(&s1)
62+
node.asDefiningArgument() instanceof AddressOfExpr
63+
}
64+
}
65+
66+
class PointerArithmeticOrArrayExpr extends Expr {
67+
Expr operand;
68+
69+
PointerArithmeticOrArrayExpr() {
70+
operand = this.(ArrayExpr).getArrayBase()
71+
or
72+
operand = this.(ArrayExpr).getArrayOffset()
73+
or
74+
operand = this.(PointerAddExpr).getAnOperand()
75+
or
76+
operand = this.(PointerSubExpr).getAnOperand()
77+
or
78+
operand = this.(Operation).getAnOperand() and
79+
operand.getUnderlyingType() instanceof PointerType and
80+
(
81+
this instanceof PostfixCrementOperation
82+
or
83+
this instanceof PrefixIncrExpr
84+
or
85+
this instanceof PrefixDecrExpr
86+
)
87+
}
88+
89+
/**
90+
* Gets the operands of this expression. If the expression is an
91+
* `ArrayExpr`, the results are the array base and offset `Expr`s.
92+
*/
93+
Expr getPointerOperand() {
94+
result = operand or
95+
result = this.(PointerArithmeticOrArrayExpr).getPointerOperand()
96+
}
97+
98+
/**
99+
* Holds if there exists an operand that is a `Literal` with a value of `0`.
100+
*/
101+
predicate isNonPointerOperandZero() { operand.(Literal).getValue().toInt() = 0 }
102+
}
103+
104+
from DataFlow::PathNode source, DataFlow::PathNode sink
105+
where
106+
not isExcluded(sink.getNode().asExpr(),
107+
InvalidMemory2Package::doNotUsePointerArithmeticOnNonArrayObjectPointersQuery()) and
108+
any(NonArrayPointerToArrayIndexingExprConfig cfg).hasFlowPath(source, sink)
109+
select sink, source, sink, "Pointer arithmetic on non-array object pointer."

‎c/cert/test/rules/ARR37-C/DoNotUsePointerArithmeticOnNonArrayObjectPointers.expected

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
edges
2+
| test.c:14:38:14:39 | p1 | test.c:18:10:18:11 | v1 |
3+
| test.c:14:38:14:39 | p1 | test.c:19:10:19:11 | v2 |
4+
| test.c:14:38:14:39 | p1 | test.c:20:10:20:11 | p1 |
5+
| test.c:14:38:14:39 | p1 | test.c:21:10:21:11 | p1 |
6+
| test.c:14:38:14:39 | p1 | test.c:22:9:22:10 | p1 |
7+
| test.c:14:38:14:39 | p1 | test.c:23:13:23:14 | p1 |
8+
| test.c:14:38:14:39 | p1 | test.c:24:9:24:10 | p1 |
9+
| test.c:14:38:14:39 | p1 | test.c:25:9:25:10 | p1 |
10+
| test.c:51:30:51:38 | & ... | test.c:14:38:14:39 | p1 |
11+
nodes
12+
| test.c:14:38:14:39 | p1 | semmle.label | p1 |
13+
| test.c:18:10:18:11 | v1 | semmle.label | v1 |
14+
| test.c:19:10:19:11 | v2 | semmle.label | v2 |
15+
| test.c:20:10:20:11 | p1 | semmle.label | p1 |
16+
| test.c:21:10:21:11 | p1 | semmle.label | p1 |
17+
| test.c:22:9:22:10 | p1 | semmle.label | p1 |
18+
| test.c:23:13:23:14 | p1 | semmle.label | p1 |
19+
| test.c:24:9:24:10 | p1 | semmle.label | p1 |
20+
| test.c:25:9:25:10 | p1 | semmle.label | p1 |
21+
| test.c:39:11:39:19 | & ... | semmle.label | & ... |
22+
| test.c:40:10:40:18 | & ... | semmle.label | & ... |
23+
| test.c:42:10:42:15 | & ... | semmle.label | & ... |
24+
| test.c:43:10:43:15 | & ... | semmle.label | & ... |
25+
| test.c:44:10:44:15 | & ... | semmle.label | & ... |
26+
| test.c:46:10:46:15 | & ... | semmle.label | & ... |
27+
| test.c:51:30:51:38 | & ... | semmle.label | & ... |
28+
subpaths
29+
#select
30+
| test.c:18:10:18:11 | v1 | test.c:51:30:51:38 | & ... | test.c:18:10:18:11 | v1 | Pointer arithmetic on non-array object pointer. |
31+
| test.c:19:10:19:11 | v2 | test.c:51:30:51:38 | & ... | test.c:19:10:19:11 | v2 | Pointer arithmetic on non-array object pointer. |
32+
| test.c:20:10:20:11 | p1 | test.c:51:30:51:38 | & ... | test.c:20:10:20:11 | p1 | Pointer arithmetic on non-array object pointer. |
33+
| test.c:21:10:21:11 | p1 | test.c:51:30:51:38 | & ... | test.c:21:10:21:11 | p1 | Pointer arithmetic on non-array object pointer. |
34+
| test.c:22:9:22:10 | p1 | test.c:51:30:51:38 | & ... | test.c:22:9:22:10 | p1 | Pointer arithmetic on non-array object pointer. |
35+
| test.c:23:13:23:14 | p1 | test.c:51:30:51:38 | & ... | test.c:23:13:23:14 | p1 | Pointer arithmetic on non-array object pointer. |
36+
| test.c:24:9:24:10 | p1 | test.c:51:30:51:38 | & ... | test.c:24:9:24:10 | p1 | Pointer arithmetic on non-array object pointer. |
37+
| test.c:25:9:25:10 | p1 | test.c:51:30:51:38 | & ... | test.c:25:9:25:10 | p1 | Pointer arithmetic on non-array object pointer. |
38+
| test.c:39:11:39:19 | & ... | test.c:39:11:39:19 | & ... | test.c:39:11:39:19 | & ... | Pointer arithmetic on non-array object pointer. |
39+
| test.c:40:10:40:18 | & ... | test.c:40:10:40:18 | & ... | test.c:40:10:40:18 | & ... | Pointer arithmetic on non-array object pointer. |
40+
| test.c:42:10:42:15 | & ... | test.c:42:10:42:15 | & ... | test.c:42:10:42:15 | & ... | Pointer arithmetic on non-array object pointer. |
41+
| test.c:43:10:43:15 | & ... | test.c:43:10:43:15 | & ... | test.c:43:10:43:15 | & ... | Pointer arithmetic on non-array object pointer. |
42+
| test.c:44:10:44:15 | & ... | test.c:44:10:44:15 | & ... | test.c:44:10:44:15 | & ... | Pointer arithmetic on non-array object pointer. |
43+
| test.c:46:10:46:15 | & ... | test.c:46:10:46:15 | & ... | test.c:46:10:46:15 | & ... | Pointer arithmetic on non-array object pointer. |

‎c/cert/test/rules/ARR37-C/DoNotUsePointerArithmeticOnNonArrayObjectPointers.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
rules/ARR37-C/DoNotUsePointerArithmeticOnNonArrayObjectPointers.ql

‎c/cert/test/rules/ARR37-C/test.c

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
struct s1 {
2+
int f1;
3+
int f2;
4+
int f3;
5+
int f4[2];
6+
};
7+
8+
struct s2 {
9+
int f1;
10+
int f2;
11+
int data[];
12+
};
13+
14+
void test_ptr_arithmetic_nested(int *p1) {
15+
// path-dependent
16+
int *v1 = p1;
17+
int *v2 = p1;
18+
(void)(v1++);
19+
(void)(v2--);
20+
(void)(p1 + 1);
21+
(void)(p1 - 1);
22+
(void)p1[1];
23+
(void)(1 [p1]);
24+
(void)p1[*p1 + 0];
25+
(void)p1[0 + 1];
26+
(void)p1[0]; // COMPLIANT
27+
}
28+
29+
void test(struct s1 p1, struct s1 *p2, struct s2 *p3) {
30+
struct s1 v1[3];
31+
struct s2 v2[3];
32+
33+
(void)*(v1 + 2); // COMPLIANT
34+
(void)*(v1 + 2); // COMPLIANT
35+
36+
(void)v1[2]; // COMPLIANT
37+
(void)v2[2]; // COMPLIANT
38+
39+
(void)((&v1[0].f1)[1]); // NON_COMPLIANT
40+
(void)(&v1[0].f1 + v1[1].f1); // NON_COMPLIANT
41+
42+
(void)(&p1.f1)[1]; // NON_COMPLIANT
43+
(void)(&p1.f1 + 1); // NON_COMPLIANT
44+
(void)(&p1.f2 - 1); // NON_COMPLIANT
45+
46+
(void)(&p1.f1 + p1.f1); // NON_COMPLIANT
47+
(void)(p1.f4 + 1); // COMPLIANT
48+
(void)(&p1.f4 + 1); // COMPLIANT
49+
50+
test_ptr_arithmetic_nested((int *)&v1[0].f4); // COMPLIANT
51+
test_ptr_arithmetic_nested(&v1[0].f1); // NON_COMPLIANT
52+
}

0 commit comments

Comments
 (0)