Skip to content

Package Preprocessor 5 #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Jul 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .vscode/tasks.json
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@
"Preprocessor2",
"Preprocessor3",
"Preprocessor4",
"Preprocessor5",
"IntegerConversion",
"Expressions",
"DeadCode",
Expand Down
112 changes: 112 additions & 0 deletions c/cert/src/rules/MSC38-C/DoNotTreatAPredefinedIdentifierAsObject.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
# MSC38-C: Do not treat a predefined identifier as an object if it might only be implemented as a macro

This query implements the CERT-C rule MSC38-C:

> Do not treat a predefined identifier as an object if it might only be implemented as a macro


## Description

The C Standard, 7.1.4 paragraph 1, \[[ISO/IEC 9899:2011](https://wiki.sei.cmu.edu/confluence/display/c/AA.+Bibliography#AA.Bibliography-ISO%2FIEC9899-2011)\] states

> Any function declared in a header may be additionally implemented as a function-like macro defined in the header, so if a library function is declared explicitly when its header is included, one of the techniques shown below can be used to ensure the declaration is not affected by such a macro. Any macro definition of a function can be suppressed locally by enclosing the name of the function in parentheses, because the name is then not followed by the left parenthesis that indicates expansion of a macro function name. For the same syntactic reason, it is permitted to take the address of a library function even if it is also defined as a macro.<sup>185</sup>


185. This means that an implementation shall provide an actual function for each library function, even if it also provides a macro for that function.

However, the C Standard enumerates specific exceptions in which the behavior of accessing an object or function expanded to be a standard library macro definition is [undefined](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-undefinedbehavior). The macros are `assert`, `errno`, `math_errhandling`, `setjmp`, `va_arg`, `va_copy`, `va_end`, and `va_start`. These cases are described by [undefined behaviors](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-undefinedbehavior) [110](https://wiki.sei.cmu.edu/confluence/display/c/CC.+Undefined+Behavior#CC.UndefinedBehavior-ub_110), [114](https://wiki.sei.cmu.edu/confluence/display/c/CC.+Undefined+Behavior#CC.UndefinedBehavior-ub_114), [122](https://wiki.sei.cmu.edu/confluence/display/c/CC.+Undefined+Behavior#CC.UndefinedBehavior-ub_122), [124](https://wiki.sei.cmu.edu/confluence/display/c/CC.+Undefined+Behavior#CC.UndefinedBehavior-ub_124), and [138](https://wiki.sei.cmu.edu/confluence/display/c/CC.+Undefined+Behavior#CC.UndefinedBehavior-ub_138). Programmers must not suppress these macros to access the underlying object or function.

## Noncompliant Code Example (assert)

In this noncompliant code example, the standard `assert()` macro is suppressed in an attempt to pass it as a function pointer to the `execute_handler()` function. Attempting to suppress the `assert()` macro is [undefined behavior](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-undefinedbehavior).

```cpp
#include <assert.h>

typedef void (*handler_type)(int);

void execute_handler(handler_type handler, int value) {
handler(value);
}

void func(int e) {
execute_handler(&(assert), e < 0);
}
```

## Compliant Solution (assert)

In this compliant solution, the `assert()` macro is wrapped in a helper function, removing the [undefined behavior](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-undefinedbehavior):

```cpp
#include <assert.h>

typedef void (*handler_type)(int);

void execute_handler(handler_type handler, int value) {
handler(value);
}

static void assert_handler(int value) {
assert(value);
}

void func(int e) {
execute_handler(&assert_handler, e < 0);
}
```

## Noncompliant Code Example (Redefining errno)

Legacy code is apt to include an incorrect declaration, such as the following in this noncompliant code example:

```cpp
extern int errno;

```

## Compliant Solution (Declaring errno)

This compliant solution demonstrates the correct way to declare `errno` by including the header `<errno.h>`:

```cpp
#include <errno.h>

```
[C-conforming](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-conformingprogram) [implementations](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-implementation) are required to declare `errno` in `<errno.h>`, although some historic implementations failed to do so.

## Risk Assessment

Accessing objects or functions underlying the specific macros enumerated in this rule is [undefined behavior](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-undefinedbehavior).

<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> MSC38-C </td> <td> Low </td> <td> Unlikely </td> <td> Medium </td> <td> <strong>P2</strong> </td> <td> <strong>L3</strong> </td> </tr> </tbody> </table>


## Automated Detection

<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, but no explicit checker </td> </tr> <tr> <td> <a> CodeSonar </a> </td> <td> 7.0p0 </td> <td> <strong>BADMACRO.STDARG_H</strong> </td> <td> Use of &lt;stdarg.h&gt; Feature </td> </tr> <tr> <td> <a> Helix QAC </a> </td> <td> 2022.2 </td> <td> <strong>C3437, C3475</strong> <strong>C++3127, C++5039</strong> </td> <td> </td> </tr> <tr> <td> <a> Parasoft C/C++test </a> </td> <td> 2022.1 </td> <td> <strong>CERT_C-MSC38-a</strong> </td> <td> A function-like macro shall not be invoked without all of its arguments </td> </tr> <tr> <td> <a> Polyspace Bug Finder </a> </td> <td> R2022a </td> <td> <a> CERT C: Rule MSC38-C </a> </td> <td> Checks for predefined macro used as an object (rule fully covered) </td> </tr> <tr> <td> <a> PRQA QA-C </a> </td> <td> 9.7 </td> <td> <strong>3437, 3475</strong> </td> <td> </td> </tr> <tr> <td> <a> RuleChecker </a> </td> <td> 22.04 </td> <td> </td> <td> Supported, but no explicit checker </td> </tr> </tbody> </table>


## Related Vulnerabilities

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+MSC38-C).

## Related Guidelines

[Key here](https://wiki.sei.cmu.edu/confluence/display/c/How+this+Coding+Standard+is+Organized#HowthisCodingStandardisOrganized-RelatedGuidelines) (explains table format and definitions)

<table> <tbody> <tr> <th> Taxonomy </th> <th> Taxonomy item </th> <th> Relationship </th> </tr> <tr> <td> <a> CERT C </a> </td> <td> <a> DCL37-C. Do not declare or define a reserved identifier </a> </td> <td> Prior to 2018-01-12: CERT: Unspecified Relationship </td> </tr> </tbody> </table>


## Bibliography

<table> <tbody> <tr> <td> <a> ISO/IEC 9899:2011 </a> </td> <td> 7.1.4, "Use of Library Functions" </td> </tr> </tbody> </table>


## Implementation notes

This query reports locations corresponding to both redefinitions of those standard library macros as well as locations where the identifiers used for accesses.

## References

* CERT-C: [MSC38-C: Do not treat a predefined identifier as an object if it might only be implemented as a macro](https://wiki.sei.cmu.edu/confluence/display/c)
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/**
* @id c/cert/do-not-treat-a-predefined-identifier-as-object
* @name MSC38-C: Do not treat a predefined identifier as an object if it might only be implemented as a macro
* @description Accessing an object or function that expands to one of a few specific standard
* library macros is undefined behaviour.
* @kind problem
* @precision very-high
* @problem.severity warning
* @tags external/cert/id/msc38-c
* correctness
* readability
* external/cert/obligation/rule
*/

import cpp
import codingstandards.c.cert

predicate hasRestrictedMacroName(string s) {
s = "assert"
or
s = "errno"
or
s = "math_errhandling"
or
s = "setjmp"
or
s = "va_arg"
or
s = "va_copy"
or
s = "va_end"
or
s = "va_start"
}

from Element m, string name
where
not isExcluded(m, Preprocessor5Package::doNotTreatAPredefinedIdentifierAsObjectQuery()) and
(
m.(Access).getTarget().hasName(name)
or
m.(Declaration).hasGlobalName(name)
) and
hasRestrictedMacroName(name)
select m, "Supression of standard library macro " + name + "."
89 changes: 89 additions & 0 deletions c/cert/src/rules/PRE32-C/MacroOrFunctionArgsContainHashToken.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# PRE32-C: Do not use preprocessor directives in invocations of function-like macros

This query implements the CERT-C rule PRE32-C:

> Do not use preprocessor directives in invocations of function-like macros


## Description

The arguments to a macro must not include preprocessor directives, such as `#define`, `#ifdef`, and `#include`. Doing so results in [undefined behavior](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-undefinedbehavior), according to the C Standard, 6.10.3, paragraph 11 \[[ISO/IEC 9899:2011](https://wiki.sei.cmu.edu/confluence/display/c/AA.+Bibliography#AA.Bibliography-ISO-IEC9899-2011)\]:

> The sequence of preprocessing tokens bounded by the outside-most matching parentheses forms the list of arguments for the function-like macro. The individual arguments within the list are separated by comma preprocessing tokens, but comma preprocessing tokens between matching inner parentheses do not separate arguments. **If there are sequences of preprocessing tokens within the list of arguments that would otherwise act as preprocessing directives, the behavior is undefined.**


See also [undefined behavior 93](https://wiki.sei.cmu.edu/confluence/display/c/CC.+Undefined+Behavior#CC.UndefinedBehavior-ub_93).

This rule also applies to the use of preprocessor directives in arguments to any function where it is unknown whether or not the function is implemented using a macro. This includes all standard library functions, such as `memcpy()`, `printf()`, and `assert()`, because any standard library function may be implemented as a macro. (C11, 7.1.4, paragraph 1).

## Noncompliant Code Example

In this noncompliant code example \[[GCC Bugs](http://gcc.gnu.org/bugs.html#nonbugs_c)\], the programmer uses preprocessor directives to specify platform-specific arguments to `memcpy()`. However, if `memcpy()` is implemented using a macro, the code results in undefined behavior.

```cpp
#include <string.h>

void func(const char *src) {
/* Validate the source string; calculate size */
char *dest;
/* malloc() destination string */
memcpy(dest, src,
#ifdef PLATFORM1
12
#else
24
#endif
);
/* ... */
}

```

## Compliant Solution

In this compliant solution \[[GCC Bugs](http://gcc.gnu.org/bugs.html#nonbugs_c)\], the appropriate call to `memcpy()` is determined outside the function call:

```cpp
#include <string.h>

void func(const char *src) {
/* Validate the source string; calculate size */
char *dest;
/* malloc() destination string */
#ifdef PLATFORM1
memcpy(dest, src, 12);
#else
memcpy(dest, src, 24);
#endif
/* ... */
}
```

## Risk Assessment

Including preprocessor directives in macro arguments is undefined behavior.

<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> PRE32-C </td> <td> Low </td> <td> Unlikely </td> <td> Medium </td> <td> <strong>P2</strong> </td> <td> <strong>L3</strong> </td> </tr> </tbody> </table>


## Automated Detection

<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> <strong>macro-argument-hash</strong> </td> <td> Fully checked </td> </tr> <tr> <td> <a> Axivion Bauhaus Suite </a> </td> <td> 7.2.0 </td> <td> <strong>CertC-PRE32</strong> </td> <td> Fully implemented </td> </tr> <tr> <td> <a> CodeSonar </a> </td> <td> 7.0p0 </td> <td> <strong>LANG.PREPROC.MACROARG</strong> </td> <td> Preprocessing directives in macro argument </td> </tr> <tr> <td> <a> ECLAIR </a> </td> <td> 1.2 </td> <td> <strong>CC2.PRE32</strong> </td> <td> Fully implemented </td> </tr> <tr> <td> <a> Helix QAC </a> </td> <td> 2022.2 </td> <td> <strong>C0853</strong> <strong>C++1072</strong> </td> <td> </td> </tr> <tr> <td> <a> Klocwork </a> </td> <td> 2022.2 </td> <td> <strong>MISRA.EXPANSION.DIRECTIVE</strong> </td> <td> </td> </tr> <tr> <td> <a> LDRA tool suite </a> </td> <td> 9.7.1 </td> <td> <strong>341 S</strong> </td> <td> Fully implemented </td> </tr> <tr> <td> <a> Parasoft C/C++test </a> </td> <td> 2022.1 </td> <td> <strong>CERT_C-PRE32-a</strong> </td> <td> Arguments to a function-like macro shall not contain tokens that look like preprocessing directives </td> </tr> <tr> <td> <a> PC-lint Plus </a> </td> <td> 1.4 </td> <td> <strong>436, 9501</strong> </td> <td> Fully supported </td> </tr> <tr> <td> <a> Polyspace Bug Finder </a> </td> <td> R2022a </td> <td> <a> CERT C: Rule PRE32-C </a> </td> <td> Checks for preprocessor directive in macro argument (rule fully covered) </td> </tr> <tr> <td> <a> PRQA QA-C </a> </td> <td> 9.7 </td> <td> <strong>0853</strong> </td> <td> </td> </tr> <tr> <td> <a> PRQA QA-C++ </a> </td> <td> 4.4 </td> <td> <strong>1072 </strong> </td> <td> </td> </tr> <tr> <td> <a> RuleChecker </a> </td> <td> 22.04 </td> <td> <strong>macro-argument-hash</strong> </td> <td> Fully checked </td> </tr> </tbody> </table>


## Related Vulnerabilities

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+PRE32-C).

## Bibliography

<table> <tbody> <tr> <td> \[ <a> GCC Bugs </a> \] </td> <td> "Non-bugs" </td> </tr> <tr> <td> \[ <a> ISO/IEC 9899:2011 </a> \] </td> <td> 6.10.3, "Macro Replacement" </td> </tr> </tbody> </table>


## Implementation notes

This query defines end of function call as the next node in the control flow graph.

## References

* CERT-C: [PRE32-C: Do not use preprocessor directives in invocations of function-like macros](https://wiki.sei.cmu.edu/confluence/display/c)
63 changes: 63 additions & 0 deletions c/cert/src/rules/PRE32-C/MacroOrFunctionArgsContainHashToken.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/**
* @id c/cert/macro-or-function-args-contain-hash-token
* @name PRE32-C: Do not use preprocessor directives in invocations of function-like macros
* @description Arguments to a function-like macros shall not contain tokens that look like
* pre-processing directives or else behaviour after macro expansion is unpredictable.
* This rule applies to functions as well in case they are implemented using macros.
* @kind problem
* @precision high
* @problem.severity warning
* @tags external/cert/id/pre32-c
* correctness
* readability
* external/cert/obligation/rule
*/

import cpp
import codingstandards.c.cert
import codingstandards.cpp.PreprocessorDirective

pragma[noinline]
predicate isFunctionInvocationLocation(FunctionCall call, File f, int startline, int endline) {
call.getLocation().hasLocationInfo(f.getAbsolutePath(), startline, _, _, _) and
isFunctionSuccessorLocation(call.getASuccessor(), f, endline)
}

pragma[noinline]
predicate isFunctionSuccessorLocation(ControlFlowNode node, File f, int endline) {
//for all function calls the closest location heurisitc we have for end line is the next node in cfg
node.getLocation().hasLocationInfo(f.getAbsolutePath(), endline, _, _, _)
}

PreprocessorDirective isLocatedInAFunctionInvocation(FunctionCall c) {
exists(PreprocessorDirective p, File f, int startCall, int endCall |
isFunctionInvocationLocation(c, f, startCall, endCall) and
exists(int startLine, int endLine | isPreprocDirectiveLine(p, f, startLine, endLine) |
startCall < startLine and
startCall < endLine and
endLine <= endCall and
endLine <= endCall
) and
result = p
)
}

from PreprocessorDirective p, string msg
where
not isExcluded(p, Preprocessor5Package::macroOrFunctionArgsContainHashTokenQuery()) and
exists(FunctionCall c |
p = isLocatedInAFunctionInvocation(c) and
//if this is actually a function in a macro ignore it because it will still be seen in the macro condition
not c.isInMacroExpansion() and
msg =
"Invocation of function " + c.getTarget().getName() + " includes a token \"" + p +
"\" that could be confused for an argument preprocessor directive."
)
or
exists(MacroInvocation m |
p = isLocatedInAMacroInvocation(m) and
msg =
"Invocation of macro " + m.getMacroName() + " includes a token \"" + p +
"\" that could be confused for an argument preprocessor directive."
)
select p, msg
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
| assert.h:2:6:2:11 | assert | Supression of standard library macro assert. |
| test.c:3:12:3:16 | errno | Supression of standard library macro errno. |
| test.c:10:21:10:26 | assert | Supression of standard library macro assert. |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
rules/MSC38-C/DoNotTreatAPredefinedIdentifierAsObject.ql
4 changes: 4 additions & 0 deletions c/cert/test/rules/MSC38-C/assert.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#undef assert
void assert(int i); // NON_COMPLIANT

#define assert(x) (void)0
17 changes: 17 additions & 0 deletions c/cert/test/rules/MSC38-C/test.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#include "assert.h"

extern int errno; // NON_COMPLIANT

typedef void (*handler_type)(int);

void execute_handler(handler_type handler, int value) { handler(value); }

void f(int e) {
execute_handler(&(assert), e < 0); // NON_COMPLIANT
}

static void assert_handler(int value) { assert(value); }

void f2(int e) {
execute_handler(&assert_handler, e < 0); // COMPLIANT
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
| test.c:9:1:9:19 | #if NOTDEFINEDMACRO | Invocation of macro MACROFUNCTION includes a token "#if NOTDEFINEDMACRO" that could be confused for an argument preprocessor directive. |
| test.c:11:1:11:5 | #else | Invocation of macro MACROFUNCTION includes a token "#else" that could be confused for an argument preprocessor directive. |
| test.c:13:1:13:6 | #endif | Invocation of macro MACROFUNCTION includes a token "#endif" that could be confused for an argument preprocessor directive. |
| test.c:20:1:20:16 | #ifdef SOMEMACRO | Invocation of function memcpy includes a token "#ifdef SOMEMACRO" that could be confused for an argument preprocessor directive. |
| test.c:22:1:22:5 | #else | Invocation of function memcpy includes a token "#else" that could be confused for an argument preprocessor directive. |
| test.c:24:1:24:6 | #endif | Invocation of function memcpy includes a token "#endif" that could be confused for an argument preprocessor directive. |
| test.c:27:1:27:8 | #if TEST | Invocation of function memcpy includes a token "#if TEST" that could be confused for an argument preprocessor directive. |
| test.c:28:1:28:6 | #endif | Invocation of function memcpy includes a token "#endif" that could be confused for an argument preprocessor directive. |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
rules/PRE32-C/MacroOrFunctionArgsContainHashToken.ql
29 changes: 29 additions & 0 deletions c/cert/test/rules/PRE32-C/test.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@

#include <string.h>
#define MACROFUNCTION(X) strlen(X)

void func(const char *src) {
char *dest;

MACROFUNCTION(
#if NOTDEFINEDMACRO // NON_COMPLIANT
"longstringtest!test!"
#else // NON_COMPLIANT
"shortstring"
#endif // NON_COMPLIANT
);

MACROFUNCTION("alright"); // COMPLIANT
memcpy(dest, src, 12); // COMPLIANT

memcpy(dest, src,
#ifdef SOMEMACRO // NON_COMPLIANT
12
#else // NON_COMPLIANT
24
#endif // NON_COMPLIANT
);

#if TEST // COMPLIANT[FALSE_POSITIVE]
#endif // COMPLIANT[FALSE_POSITIVE]
}
Loading