Skip to content

Commit 49cded3

Browse files
lcarteylcartey
committed
PRE31-C: Implement query
Implements a heuristic query to find cases where arguments are provided to unsafe macros which have side-effects.
1 parent 9822ec6 commit 49cded3

10 files changed

Lines changed: 185 additions & 2 deletions

File tree

‎c/cert/src/rules/PRE31-C/SideEffectsInArgumentsToUnsafeMacros.md‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# PRE31-C: Avoid side effects in arguments to unsafe macros
2+
3+
This query implements the CERT-C rule PRE31-C:
4+
5+
> Avoid side effects in arguments to unsafe macros
6+
## CERT
7+
8+
** REPLACE THIS BY RUNNING THE SCRIPT `scripts/help/cert-help-extraction.py` **
9+
10+
## Implementation notes
11+
12+
None
13+
14+
## References
15+
16+
* CERT-C: [PRE31-C: Avoid side effects in arguments to unsafe macros](https://wiki.sei.cmu.edu/confluence/display/c)

‎c/cert/src/rules/PRE31-C/SideEffectsInArgumentsToUnsafeMacros.ql‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/**
2+
* @id c/cert/side-effects-in-arguments-to-unsafe-macros
3+
* @name PRE31-C: Avoid side effects in arguments to unsafe macros
4+
* @description Macro arguments can be expanded multiple times which can cause side-effects to be
5+
* evaluated multiple times.
6+
* @kind problem
7+
* @precision low
8+
* @problem.severity error
9+
* @tags external/cert/id/pre31-c
10+
* correctness
11+
* external/cert/obligation/rule
12+
*/
13+
14+
import cpp
15+
import codingstandards.c.cert
16+
import codingstandards.cpp.Macro
17+
import codingstandards.cpp.SideEffect
18+
import codingstandards.cpp.StructuralEquivalence
19+
import codingstandards.cpp.sideeffect.DefaultEffects
20+
import codingstandards.cpp.sideeffect.Customizations
21+
22+
class FunctionCallEffect extends GlobalSideEffect::Range {
23+
FunctionCallEffect() {
24+
exists(Function f |
25+
f = this.(FunctionCall).getTarget() and
26+
// Not a side-effecting function
27+
not f.(BuiltInFunction).getName() = "__builtin_expect" and
28+
// Not side-effecting functions
29+
not exists(string name |
30+
name =
31+
[
32+
"acos", "asin", "atan", "atan2", "ceil", "cos", "cosh", "exp", "fabs", "floor", "fmod",
33+
"frexp", "ldexp", "log", "log10", "modf", "pow", "sin", "sinh", "sqrt", "tan", "tanh",
34+
"cbrt", "erf", "erfc", "exp2", "expm1", "fdim", "fma", "fmax", "fmin", "hypot", "ilogb",
35+
"lgamma", "llrint", "llround", "log1p", "log2", "logb", "lrint", "lround", "nan",
36+
"nearbyint", "nextafter", "nexttoward", "remainder", "remquo", "rint", "round",
37+
"scalbln", "scalbn", "tgamma", "trunc"
38+
] and
39+
f.hasGlobalOrStdName([name, name + "f", name + "l"])
40+
)
41+
)
42+
}
43+
}
44+
45+
class CrementEffect extends LocalSideEffect::Range {
46+
CrementEffect() { this instanceof CrementOperation }
47+
}
48+
49+
from
50+
FunctionLikeMacro flm, MacroInvocation mi, Expr e, SideEffect sideEffect, int i, string arg,
51+
string sideEffectDesc
52+
where
53+
not isExcluded(e, SideEffects4Package::sideEffectsInArgumentsToUnsafeMacrosQuery()) and
54+
sideEffect = getASideEffect(e) and
55+
flm.getAnInvocation() = mi and
56+
not exists(mi.getParentInvocation()) and
57+
mi.getAnExpandedElement() = e and
58+
// Only consider arguments that are expanded multiple times, and do not consider "stringified" arguments
59+
count(int index | index = flm.getAParameterUse(i) and not flm.getBody().charAt(index) = "#") > 1 and
60+
arg = mi.getExpandedArgument(i) and
61+
(
62+
sideEffect instanceof CrementEffect and
63+
exists(arg.indexOf(sideEffect.(CrementOperation).getOperator())) and
64+
sideEffectDesc = "the use of the " + sideEffect.(CrementOperation).getOperator() + " operator"
65+
or
66+
sideEffect instanceof FunctionCallEffect and
67+
exists(arg.indexOf(sideEffect.(FunctionCall).getTarget().getName() + "(")) and
68+
sideEffectDesc =
69+
"a call to the function '" + sideEffect.(FunctionCall).getTarget().getName() + "'"
70+
)
71+
select sideEffect,
72+
"Argument " + mi.getUnexpandedArgument(i) + " to unsafe macro '" + flm.getName() +
73+
"' is expanded to '" + arg + "' multiple times and includes " + sideEffectDesc +
74+
" as a side-effect."

‎c/cert/test/rules/PRE31-C/SideEffectsInArgumentsToUnsafeMacros.expected‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
| test.c:9:10:9:12 | ... ++ | Argument i++ to unsafe macro 'unsafe' is expanded to 'i++' multiple times and includes the use of the ++ operator as a side-effect. |
2+
| test.c:9:10:9:12 | ... ++ | Argument i++ to unsafe macro 'unsafe' is expanded to 'i++' multiple times and includes the use of the ++ operator as a side-effect. |
3+
| test.c:11:10:11:12 | ... -- | Argument i-- to unsafe macro 'unsafe' is expanded to 'i--' multiple times and includes the use of the -- operator as a side-effect. |
4+
| test.c:11:10:11:12 | ... -- | Argument i-- to unsafe macro 'unsafe' is expanded to 'i--' multiple times and includes the use of the -- operator as a side-effect. |
5+
| test.c:26:10:26:15 | call to addOne | Argument addOne(10) to unsafe macro 'unsafe' is expanded to 'addOne(10)' multiple times and includes a call to the function 'addOne' as a side-effect. |
6+
| test.c:26:10:26:15 | call to addOne | Argument addOne(10) to unsafe macro 'unsafe' is expanded to 'addOne(10)' multiple times and includes a call to the function 'addOne' as a side-effect. |
7+
| test.c:27:10:27:17 | call to external | Argument external() to unsafe macro 'unsafe' is expanded to 'external()' multiple times and includes a call to the function 'external' as a side-effect. |
8+
| test.c:27:10:27:17 | call to external | Argument external() to unsafe macro 'unsafe' is expanded to 'external()' multiple times and includes a call to the function 'external' as a side-effect. |
9+
| test.c:28:10:28:15 | call to writeX | Argument writeX(10) to unsafe macro 'unsafe' is expanded to 'writeX(10)' multiple times and includes a call to the function 'writeX' as a side-effect. |
10+
| test.c:28:10:28:15 | call to writeX | Argument writeX(10) to unsafe macro 'unsafe' is expanded to 'writeX(10)' multiple times and includes a call to the function 'writeX' as a side-effect. |

‎c/cert/test/rules/PRE31-C/SideEffectsInArgumentsToUnsafeMacros.qlref‎

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

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
#include <stdio.h>
2+
3+
#define safe(x) ((x) + 1)
4+
#define unsafe(x) (x) * (x)
5+
6+
void test_crement() {
7+
int i = 0;
8+
safe(i++); // COMPLIANT
9+
unsafe(i++); // NON_COMPLIANT
10+
safe(i--); // COMPLIANT
11+
unsafe(i--); // NON_COMPLIANT
12+
}
13+
14+
int addOne(int x) { return x + 1; }
15+
int writeX(int x) {
16+
printf("%d", x);
17+
return x;
18+
}
19+
20+
int external();
21+
22+
void test_call() {
23+
safe(addOne(10)); // COMPLIANT
24+
safe(external()); // COMPLIANT
25+
safe(writeX(10)); // COMPLIANT
26+
unsafe(addOne(10)); // COMPLIANT
27+
unsafe(external()); // NON_COMPLIANT
28+
unsafe(writeX(10)); // NON_COMPLIANT
29+
}

‎cpp/common/src/codingstandards/cpp/Macro.qll‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ class FunctionLikeMacro extends Macro {
1515

1616
int getAParameterUse(int index) {
1717
exists(string parameter | parameter = getParameter(index) |
18-
result = this.getBody().indexOf(parameter)
18+
// Find identifier tokens in the program that match the parameter name
19+
exists(this.getBody().regexpFind("\\#?\\b" + parameter + "\\b", _, result))
1920
)
2021
}
2122
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import Preprocessor5
5050
import Preprocessor6
5151
import SideEffects1
5252
import SideEffects2
53+
import SideEffects4
5354
import SignalHandlers
5455
import StandardLibraryFunctionTypes
5556
import Statements1
@@ -113,6 +114,7 @@ newtype TCQuery =
113114
TPreprocessor6PackageQuery(Preprocessor6Query q) or
114115
TSideEffects1PackageQuery(SideEffects1Query q) or
115116
TSideEffects2PackageQuery(SideEffects2Query q) or
117+
TSideEffects4PackageQuery(SideEffects4Query q) or
116118
TSignalHandlersPackageQuery(SignalHandlersQuery q) or
117119
TStandardLibraryFunctionTypesPackageQuery(StandardLibraryFunctionTypesQuery q) or
118120
TStatements1PackageQuery(Statements1Query q) or
@@ -176,6 +178,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat
176178
isPreprocessor6QueryMetadata(query, queryId, ruleId, category) or
177179
isSideEffects1QueryMetadata(query, queryId, ruleId, category) or
178180
isSideEffects2QueryMetadata(query, queryId, ruleId, category) or
181+
isSideEffects4QueryMetadata(query, queryId, ruleId, category) or
179182
isSignalHandlersQueryMetadata(query, queryId, ruleId, category) or
180183
isStandardLibraryFunctionTypesQueryMetadata(query, queryId, ruleId, category) or
181184
isStatements1QueryMetadata(query, queryId, ruleId, category) or

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/
2+
import cpp
3+
import RuleMetadata
4+
import codingstandards.cpp.exclusions.RuleMetadata
5+
6+
newtype SideEffects4Query = TSideEffectsInArgumentsToUnsafeMacrosQuery()
7+
8+
predicate isSideEffects4QueryMetadata(Query query, string queryId, string ruleId, string category) {
9+
query =
10+
// `Query` instance for the `sideEffectsInArgumentsToUnsafeMacros` query
11+
SideEffects4Package::sideEffectsInArgumentsToUnsafeMacrosQuery() and
12+
queryId =
13+
// `@id` for the `sideEffectsInArgumentsToUnsafeMacros` query
14+
"c/cert/side-effects-in-arguments-to-unsafe-macros" and
15+
ruleId = "PRE31-C" and
16+
category = "rule"
17+
}
18+
19+
module SideEffects4Package {
20+
Query sideEffectsInArgumentsToUnsafeMacrosQuery() {
21+
//autogenerate `Query` type
22+
result =
23+
// `Query` type for `sideEffectsInArgumentsToUnsafeMacros` query
24+
TQueryC(TSideEffects4PackageQuery(TSideEffectsInArgumentsToUnsafeMacrosQuery()))
25+
}
26+
}

‎rule_packages/c/SideEffects4.json‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{
2+
"CERT-C": {
3+
"PRE31-C": {
4+
"properties": {
5+
"obligation": "rule"
6+
},
7+
"queries": [
8+
{
9+
"description": "Macro arguments can be expanded multiple times which can cause side-effects to be evaluated multiple times.",
10+
"kind": "problem",
11+
"name": "Avoid side effects in arguments to unsafe macros",
12+
"precision": "low",
13+
"severity": "error",
14+
"short_name": "SideEffectsInArgumentsToUnsafeMacros",
15+
"tags": [
16+
"correctness"
17+
]
18+
}
19+
],
20+
"title": "Avoid side effects in arguments to unsafe macros"
21+
}
22+
}
23+
}

‎rules.csv‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,7 @@ c,CERT-C,POS52-C,OutOfScope,Rule,,,Do not perform operations that can block whil
586586
c,CERT-C,POS53-C,OutOfScope,Rule,,,Do not use more than one mutex for concurrent waiting operations on a condition variable,,,,
587587
c,CERT-C,POS54-C,OutOfScope,Rule,,,Detect and handle POSIX library errors,,,,
588588
c,CERT-C,PRE30-C,No,Rule,,,Do not create a universal character name through concatenation,,,Medium,
589-
c,CERT-C,PRE31-C,Yes,Rule,,,Avoid side effects in arguments to unsafe macros,RULE-13-2,SideEffects,Medium,
589+
c,CERT-C,PRE31-C,Yes,Rule,,,Avoid side effects in arguments to unsafe macros,RULE-13-2,SideEffects4,Medium,
590590
c,CERT-C,PRE32-C,Yes,Rule,,,Do not use preprocessor directives in invocations of function-like macros,,Preprocessor5,Hard,
591591
c,CERT-C,SIG30-C,Yes,Rule,,,Call only asynchronous-safe functions within signal handlers,,SignalHandlers,Medium,
592592
c,CERT-C,SIG31-C,Yes,Rule,,,Do not access shared objects in signal handlers,,SignalHandlers,Medium,

0 commit comments

Comments
 (0)