Skip to content

Commit 8240376

Browse files
mbaludambaluda
committed
IMplement EXP40-C
1 parent 266c3e5 commit 8240376

File tree

9 files changed

+144
-55
lines changed

9 files changed

+144
-55
lines changed

‎c/cert/src/rules/EXP40-C/DoNotModifyConstantObjects.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ This query implements the CERT-C rule EXP40-C:
55
> Do not modify constant objects
66
77

8-
98
## Description
109

1110
The C Standard, 6.7.3, paragraph 6 \[[IS](https://wiki.sei.cmu.edu/confluence/display/c/AA.+Bibliography#AA.Bibliography-ISO-IEC9899-2011)[O/IEC 9899:2011](https://wiki.sei.cmu.edu/confluence/display/c/AA.+Bibliography#AA.Bibliography-ISO-IEC9899-2011)\], states
@@ -89,7 +88,7 @@ Search for [vulnerabilities](https://wiki.sei.cmu.edu/confluence/display/c/BB.+D
8988

9089
## Implementation notes
9190

92-
None
91+
The implementation does not consider pointer aliasing via multiple indirection.
9392

9493
## References
9594

‎c/cert/src/rules/EXP40-C/DoNotModifyConstantObjects.ql

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,61 @@
11
/**
22
* @id c/cert/do-not-modify-constant-objects
33
* @name EXP40-C: Do not modify constant objects
4-
* @description
5-
* @kind problem
6-
* @precision very-high
4+
* @description Do not modify constant objects. This may result in undefined behavior.
5+
* @kind path-problem
6+
* @precision high
77
* @problem.severity error
88
* @tags external/cert/id/exp40-c
9+
* correctness
910
* external/cert/obligation/rule
1011
*/
1112

1213
import cpp
1314
import codingstandards.c.cert
15+
import semmle.code.cpp.dataflow.DataFlow
16+
import DataFlow::PathGraph
17+
import codingstandards.cpp.SideEffect
1418

15-
from
19+
class ConstRemovingCast extends Cast {
20+
ConstRemovingCast() {
21+
this.getExpr().getType().(DerivedType).getBaseType*().isConst() and
22+
not this.getType().(DerivedType).getBaseType*().isConst()
23+
}
24+
}
25+
26+
class MaybeReturnsStringLiteralFunctionCall extends FunctionCall {
27+
MaybeReturnsStringLiteralFunctionCall() {
28+
getTarget().getName() in [
29+
"strpbrk", "strchr", "strrchr", "strstr", "wcspbrk", "wcschr", "wcsrchr", "wcsstr",
30+
"memchr", "wmemchr"
31+
]
32+
}
33+
}
34+
35+
class MyDataFlowConfCast extends DataFlow::Configuration {
36+
MyDataFlowConfCast() { this = "MyDataFlowConfCast" }
37+
38+
override predicate isSource(DataFlow::Node source) {
39+
source.asExpr().getFullyConverted() instanceof ConstRemovingCast
40+
or
41+
source.asExpr().getFullyConverted() = any(MaybeReturnsStringLiteralFunctionCall c)
42+
}
43+
44+
override predicate isSink(DataFlow::Node sink) {
45+
sink.asExpr() = any(Assignment a).getLValue().(PointerDereferenceExpr).getOperand()
46+
}
47+
}
48+
49+
from MyDataFlowConfCast conf, DataFlow::PathNode src, DataFlow::PathNode sink
1650
where
17-
not isExcluded(x, Contracts6Package::doNotModifyConstantObjectsQuery()) and
18-
select
51+
conf.hasFlowPath(src, sink)
52+
or
53+
sink.getNode()
54+
.asExpr()
55+
.(VariableEffect)
56+
.getTarget()
57+
.getType()
58+
.(DerivedType)
59+
.getBaseType*()
60+
.isConst()
61+
select sink, src, sink, "Const variable assigned with non const-value."

‎c/cert/test/rules/EXP40-C/DoNotModifyConstantObjects.expected

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,29 @@
1-
No expected results have yet been specified
1+
edges
2+
| test.c:5:8:5:9 | & ... | test.c:6:4:6:5 | aa |
3+
| test.c:26:15:26:15 | a | test.c:27:4:27:4 | a |
4+
| test.c:34:13:34:14 | & ... | test.c:39:7:39:8 | p1 |
5+
| test.c:39:7:39:8 | p1 | test.c:26:15:26:15 | a |
6+
| test.c:40:7:40:9 | * ... | test.c:26:15:26:15 | a |
7+
| test.c:59:7:59:8 | & ... | test.c:60:4:60:4 | p |
8+
| test.c:79:11:79:16 | call to strchr | test.c:81:6:81:12 | ... ++ |
9+
nodes
10+
| test.c:5:8:5:9 | & ... | semmle.label | & ... |
11+
| test.c:6:4:6:5 | aa | semmle.label | aa |
12+
| test.c:26:15:26:15 | a | semmle.label | a |
13+
| test.c:27:4:27:4 | a | semmle.label | a |
14+
| test.c:34:13:34:14 | & ... | semmle.label | & ... |
15+
| test.c:39:7:39:8 | p1 | semmle.label | p1 |
16+
| test.c:40:7:40:9 | * ... | semmle.label | * ... |
17+
| test.c:59:7:59:8 | & ... | semmle.label | & ... |
18+
| test.c:60:4:60:4 | p | semmle.label | p |
19+
| test.c:74:12:74:12 | s | semmle.label | s |
20+
| test.c:79:11:79:16 | call to strchr | semmle.label | call to strchr |
21+
| test.c:81:6:81:12 | ... ++ | semmle.label | ... ++ |
22+
subpaths
23+
#select
24+
| test.c:6:4:6:5 | aa | test.c:5:8:5:9 | & ... | test.c:6:4:6:5 | aa | Const variable assigned with non const-value. |
25+
| test.c:27:4:27:4 | a | test.c:34:13:34:14 | & ... | test.c:27:4:27:4 | a | Const variable assigned with non const-value. |
26+
| test.c:27:4:27:4 | a | test.c:40:7:40:9 | * ... | test.c:27:4:27:4 | a | Const variable assigned with non const-value. |
27+
| test.c:60:4:60:4 | p | test.c:59:7:59:8 | & ... | test.c:60:4:60:4 | p | Const variable assigned with non const-value. |
28+
| test.c:74:12:74:12 | s | test.c:74:12:74:12 | s | test.c:74:12:74:12 | s | Const variable assigned with non const-value. |
29+
| test.c:81:6:81:12 | ... ++ | test.c:79:11:79:16 | call to strchr | test.c:81:6:81:12 | ... ++ | Const variable assigned with non const-value. |

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

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ void f1() {
22
const int a = 3;
33
int *aa;
44

5-
aa = &a; // NON_COMPLIANT
6-
*aa = 100;
5+
aa = &a;
6+
*aa = 100; // NON_COMPLIANT
77
}
88

99
void f1a() {
@@ -31,13 +31,13 @@ void f4b(int *a) {}
3131

3232
void f4() {
3333
const int a = 100;
34-
int *p1 = &a; // NON_COMPLIANT
34+
int *p1 = &a; // COMPLIANT
3535
const int **p2;
3636

37-
*p2 = &a; // NON_COMPLIANT
37+
*p2 = &a; // COMPLIANT
3838

39-
f4a(p1); // NON_COMPLIANT
40-
f4a(*p2); // NON_COMPLIANT
39+
f4a(p1); // COMPLIANT
40+
f4a(*p2); // COMPLIANT
4141
}
4242

4343
void f5() {
@@ -49,4 +49,37 @@ void f5() {
4949

5050
f4b(p1);
5151
f4b(*p2);
52+
}
53+
54+
#include <string.h>
55+
56+
void f6a() {
57+
char *p;
58+
const char c = 'A';
59+
p = &c;
60+
*p = 0; // NON_COMPLIANT
61+
}
62+
63+
void f6b() {
64+
const char **cpp;
65+
char *p;
66+
const char c = 'A';
67+
cpp = &p;
68+
*cpp = &c;
69+
*p = 0; // NON_COMPLIANT[FALSE_NEGATIVE]
70+
}
71+
72+
const char s[] = "foo"; // source
73+
void f7() {
74+
*(char *)s = '\0'; // NON_COMPLIANT
75+
}
76+
77+
const char *f8(const char *pathname) {
78+
char *slash;
79+
slash = strchr(pathname, '/');
80+
if (slash) {
81+
*slash++ = '\0'; // NON_COMPLIANT
82+
return slash;
83+
}
84+
return pathname;
5285
}

‎c/misra/src/rules/RULE-17-5/ArrayFunctionArgumentNumberOfElements.ql

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
/**
22
* @id c/misra/array-function-argument-number-of-elements
3-
* @name RULE-17-5: The function argument corresponding to a parameter declared to have an array type shall have an
4-
* @description The function argument corresponding to a parameter declared to have an array type
5-
* shall have an appropriate number of elements
3+
* @name RULE-17-5: An array founction argument shall have an appropriate number of elements
4+
* @description The function argument corresponding to an array parameter shall have an appropriate
5+
* number of elements
66
* @kind problem
77
* @precision high
88
* @problem.severity error
99
* @tags external/misra/id/rule-17-5
10+
* correctness
1011
* external/misra/obligation/advisory
1112
*/
1213

‎c/misra/src/rules/RULE-17-7/ValueReturnedByAFunctionNotUsed.ql

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
/**
22
* @id c/misra/value-returned-by-a-function-not-used
3-
* @name RULE-17-7: The value returned by a function having non-void return type shall be used
4-
* @description
3+
* @name RULE-17-7: Return values should be used or cast to void
4+
* @description The value returned by a function having non-void return type shall be used or cast
5+
* to void
56
* @kind problem
67
* @precision very-high
78
* @problem.severity error
89
* @tags external/misra/id/rule-17-7
10+
* correctness
911
* external/misra/obligation/required
1012
*/
1113

‎cpp/common/src/codingstandards/cpp/exclusions/c/Contracts6.qll

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,6 @@ predicate isContracts6QueryMetadata(Query query, string queryId, string ruleId,
1919
ruleId = "EXP40-C" and
2020
category = "rule"
2121
or
22-
query =
23-
// `Query` instance for the `rightHandOperandOfAShiftOperatorRange` query
24-
Contracts6Package::rightHandOperandOfAShiftOperatorRangeQuery() and
25-
queryId =
26-
// `@id` for the `rightHandOperandOfAShiftOperatorRange` query
27-
"c/misra/right-hand-operand-of-a-shift-operator-range" and
28-
ruleId = "RULE-12-2" and
29-
category = "required"
30-
or
3122
query =
3223
// `Query` instance for the `arrayFunctionArgumentNumberOfElements` query
3324
Contracts6Package::arrayFunctionArgumentNumberOfElementsQuery() and

‎rule_packages/c/Contracts6.json

Lines changed: 15 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,36 +6,22 @@
66
},
77
"queries": [
88
{
9-
"description": "",
10-
"kind": "problem",
9+
"description": "Do not modify constant objects. This may result in undefined behavior.",
10+
"kind": "path-problem",
1111
"name": "Do not modify constant objects",
12-
"precision": "very-high",
12+
"precision": "high",
1313
"severity": "error",
1414
"short_name": "DoNotModifyConstantObjects",
15-
"tags": []
15+
"tags": ["correctness"],
16+
"implementation_scope": {
17+
"description": "The implementation does not consider pointer aliasing via multiple indirection."
18+
}
1619
}
1720
],
1821
"title": "Do not modify constant objects"
1922
}
2023
},
2124
"MISRA-C-2012": {
22-
"RULE-12-2": {
23-
"properties": {
24-
"obligation": "required"
25-
},
26-
"queries": [
27-
{
28-
"description": "The right hand operand of a shift operator shall lie in the range zero to one less than the width in bits of the essential type of the left hand operand",
29-
"kind": "problem",
30-
"name": "The right hand operand of a shift operator shall lie in the range zero to one less than the width in",
31-
"precision": "high",
32-
"severity": "error",
33-
"short_name": "RightHandOperandOfAShiftOperatorRange",
34-
"tags": []
35-
}
36-
],
37-
"title": "The right hand operand of a shift operator shall lie in the range zero to one less than the width in bits of the essential type of the left hand operand"
38-
},
3925
"RULE-17-5": {
4026
"properties": {
4127
"obligation": "advisory"
@@ -48,7 +34,10 @@
4834
"precision": "high",
4935
"severity": "error",
5036
"short_name": "ArrayFunctionArgumentNumberOfElements",
51-
"tags": []
37+
"tags": ["correctness"],
38+
"implementation_scope": {
39+
"description": "The rule is enforced in the context of a single function."
40+
}
5241
}
5342
],
5443
"title": "The function argument corresponding to a parameter declared to have an array type shall have an appropriate number of elements"
@@ -65,7 +54,10 @@
6554
"precision": "very-high",
6655
"severity": "error",
6756
"short_name": "ValueReturnedByAFunctionNotUsed",
68-
"tags": []
57+
"tags": ["correctness"],
58+
"implementation_scope": {
59+
"description": "The rule is enforced in the context of a single function."
60+
}
6961
}
7062
],
7163
"title": "The value returned by a function having non-void return type shall be used"

‎rules.csv

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,7 @@ c,CERT-C,EXP35-C,Yes,Rule,,,Do not modify objects with temporary lifetime,,Inval
523523
c,CERT-C,EXP36-C,Yes,Rule,,,Do not cast pointers into more strictly aligned pointer types,,Pointers3,Medium,
524524
c,CERT-C,EXP37-C,Yes,Rule,,,Call functions with the correct number and type of arguments,,Expressions,Easy,
525525
c,CERT-C,EXP39-C,Yes,Rule,,,Do not access a variable through a pointer of an incompatible type,,Pointers3,Medium,
526-
c,CERT-C,EXP40-C,Yes,Rule,,,Do not modify constant objects,,Contracts6,Medium,
526+
c,CERT-C,EXP40-C,Yes,Rule,,,Do not modify constant objects,,Contracts6,Hard,
527527
c,CERT-C,EXP42-C,Yes,Rule,,,Do not compare padding data,,Memory,Medium,
528528
c,CERT-C,EXP43-C,Yes,Rule,,,Avoid undefined behavior when using restrict-qualified pointers,,Pointers3,Medium,
529529
c,CERT-C,EXP44-C,Yes,Rule,,,"Do not rely on side effects in operands to sizeof, _Alignof, or _Generic",M5-3-4,SideEffects1,Medium,
@@ -683,7 +683,7 @@ c,MISRA-C-2012,RULE-11-7,Yes,Required,,,A cast shall not be performed between po
683683
c,MISRA-C-2012,RULE-11-8,Yes,Required,,,A cast shall not remove any const or volatile qualification from the type pointed to by a pointer,,Pointers1,Easy,
684684
c,MISRA-C-2012,RULE-11-9,Yes,Required,,,The macro NULL shall be the only permitted form of integer null pointer constant,,Pointers1,Easy,
685685
c,MISRA-C-2012,RULE-12-1,Yes,Advisory,,,The precedence of operators within expressions should be made explicit,,SideEffects1,Medium,
686-
c,MISRA-C-2012,RULE-12-2,Yes,Required,,,The right hand operand of a shift operator shall lie in the range zero to one less than the width in bits of the essential type of the left hand operand,,Contracts6,Hard,
686+
c,MISRA-C-2012,RULE-12-2,Yes,Required,,,The right hand operand of a shift operator shall lie in the range zero to one less than the width in bits of the essential type of the left hand operand,,Contracts,Medium,
687687
c,MISRA-C-2012,RULE-12-3,Yes,Advisory,,,The comma operator should not be used,M5-18-1,Banned,Import,
688688
c,MISRA-C-2012,RULE-12-4,Yes,Advisory,,,Evaluation of constant expressions should not lead to unsigned integer wrap-around,INT30-C,Types,Easy,
689689
c,MISRA-C-2012,RULE-12-5,Yes,Mandatory,,,The sizeof operator shall not have an operand which is a function parameter declared as �array of type�,,Types,Medium,
@@ -717,7 +717,7 @@ c,MISRA-C-2012,RULE-17-3,Yes,Mandatory,,,A function shall not be declared implic
717717
c,MISRA-C-2012,RULE-17-4,Yes,Mandatory,,,All exit paths from a function with non-void return type shall have an explicit return statement with an expression,MSC52-CPP,Statements,Medium,
718718
c,MISRA-C-2012,RULE-17-5,Yes,Advisory,,,The function argument corresponding to a parameter declared to have an array type shall have an appropriate number of elements,,Contracts6,Hard,
719719
c,MISRA-C-2012,RULE-17-6,No,Mandatory,,,The declaration of an array parameter shall not contain the static keyword between the [ ],,,,
720-
c,MISRA-C-2012,RULE-17-7,Yes,Required,,,The value returned by a function having non-void return type shall be used,A0-1-2,Contracts6,Import,
720+
c,MISRA-C-2012,RULE-17-7,Yes,Required,,,The value returned by a function having non-void return type shall be used,A0-1-2,Contracts6,Easy,
721721
c,MISRA-C-2012,RULE-17-8,Yes,Advisory,,,A function parameter should not be modified,,SideEffects2,Medium,
722722
c,MISRA-C-2012,RULE-18-1,Yes,Required,,,A pointer resulting from arithmetic on a pointer operand shall address an element of the same array as that pointer operand,M5-0-16,Pointers1,Import,
723723
c,MISRA-C-2012,RULE-18-2,Yes,Required,,,Subtraction between pointers shall only be applied to pointers that address elements of the same array,M5-0-17,Pointers1,Import,

0 commit comments

Comments
 (0)