Skip to content

Commit 7072866

Browse files
knewbury01knewbury01
authored
Merge pull request #113 from mbaluda-org/Contracts5
Package Contracts5
2 parents 48f7ef5 + c5dcdc7 commit 7072866

15 files changed

+1491
-11
lines changed

‎c/cert/src/rules/ERR30-C/FunctionCallBeforeErrnoCheck.ql

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,6 @@ class ErrnoSettingFunctionCall extends FunctionCall {
2323
ErrnoSettingFunctionCall() { this.getTarget() instanceof InBandErrnoSettingFunction }
2424
}
2525

26-
class ErrnoCheck extends Expr {
27-
ErrnoCheck() {
28-
this = any(MacroInvocation ma | ma.getMacroName() = "errno").getAnExpandedElement()
29-
or
30-
this.(FunctionCall).getTarget().hasName(["perror", "strerror"])
31-
}
32-
}
33-
3426
/**
3527
* A successor of an ErrnoSettingFunctionCall appearing
3628
* before a check of errno
@@ -42,7 +34,7 @@ ControlFlowNode errnoNotCheckedAfter(ErrnoSettingFunctionCall errnoSet) {
4234
result = mid.getASuccessor() and
4335
mid = errnoNotCheckedAfter(errnoSet) and
4436
// stop recursion on an error check
45-
not result instanceof ErrnoCheck
37+
not result instanceof ErrnoRead
4638
)
4739
}
4840

‎c/cert/src/rules/ERR32-C/DoNotRelyOnIndeterminateValuesOfErrno.md

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
# ERR32-C: Do not rely on indeterminate values of errno
2+
3+
This query implements the CERT-C rule ERR32-C:
4+
5+
> Do not rely on indeterminate values of errno
6+
7+
8+
## Description
9+
10+
According to the C Standard \[[ISO/IEC 9899:2011](https://wiki.sei.cmu.edu/confluence/display/c/AA.+Bibliography#AA.Bibliography-ISO-IEC9899-2011)\], the behavior of a program is [undefined](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-undefinedbehavior) when
11+
12+
> the value of `errno` is referred to after a signal occurred other than as the result of calling the `abort` or `raise` function and the corresponding signal handler obtained a `SIG_ERR` return from a call to the `signal` function.
13+
14+
15+
See [undefined behavior 133](https://wiki.sei.cmu.edu/confluence/display/c/CC.+Undefined+Behavior#CC.UndefinedBehavior-ub_133).
16+
17+
A signal handler is allowed to call `signal();` if that fails, `signal()` returns `SIG_ERR` and sets `errno` to a positive value. However, if the event that caused a signal was external (not the result of the program calling `abort()` or `raise()`), the only functions the signal handler may call are `_Exit()` or `abort()`, or it may call `signal()` on the signal currently being handled; if `signal()` fails, the value of `errno` is [indeterminate](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-indeterminatevalue).
18+
19+
This rule is also a special case of [SIG31-C. Do not access shared objects in signal handlers](https://wiki.sei.cmu.edu/confluence/display/c/SIG31-C.+Do+not+access+shared+objects+in+signal+handlers). The object designated by `errno` is of static storage duration and is not a `volatile sig_atomic_t`. As a result, performing any action that would require `errno` to be set would normally cause [undefined behavior](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-undefinedbehavior). The C Standard, 7.14.1.1, paragraph 5, makes a special exception for `errno` in this case, allowing `errno` to take on an indeterminate value but specifying that there is no other [undefined behavior](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-undefinedbehavior). This special exception makes it possible to call `signal()` from within a signal handler without risking [undefined behavior](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-undefinedbehavior), but the handler, and any code executed after the handler returns, must not depend on the value of `errno` being meaningful.
20+
21+
## Noncompliant Code Example
22+
23+
The `handler()` function in this noncompliant code example attempts to restore default handling for the signal indicated by `signum`. If the request to set the signal to default can be honored, the `signal()` function returns the value of the signal handler for the most recent successful call to the `signal()` function for the specified signal. Otherwise, a value of `SIG_ERR` is returned and a positive value is stored in `errno`. Unfortunately, the value of `errno` is indeterminate because the `handler()` function is called when an external signal is raised, so any attempt to read `errno` (for example, by the `perror()` function) is [undefined behavior](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-undefinedbehavior):
24+
25+
```cpp
26+
#include <signal.h>
27+
#include <stdlib.h>
28+
#include <stdio.h>
29+
30+
typedef void (*pfv)(int);
31+
32+
void handler(int signum) {
33+
pfv old_handler = signal(signum, SIG_DFL);
34+
if (old_handler == SIG_ERR) {
35+
perror("SIGINT handler"); /* Undefined behavior */
36+
/* Handle error */
37+
}
38+
}
39+
40+
int main(void) {
41+
pfv old_handler = signal(SIGINT, handler);
42+
if (old_handler == SIG_ERR) {
43+
perror("SIGINT handler");
44+
/* Handle error */
45+
}
46+
47+
/* Main code loop */
48+
49+
return EXIT_SUCCESS;
50+
}
51+
52+
```
53+
The call to `perror()` from `handler()` also violates [SIG30-C. Call only asynchronous-safe functions within signal handlers](https://wiki.sei.cmu.edu/confluence/display/c/SIG30-C.+Call+only+asynchronous-safe+functions+within+signal+handlers).
54+
55+
## Compliant Solution
56+
57+
This compliant solution does not reference `errno` and does not return from the signal handler if the `signal()` call fails:
58+
59+
```cpp
60+
#include <signal.h>
61+
#include <stdlib.h>
62+
#include <stdio.h>
63+
64+
typedef void (*pfv)(int);
65+
66+
void handler(int signum) {
67+
pfv old_handler = signal(signum, SIG_DFL);
68+
if (old_handler == SIG_ERR) {
69+
abort();
70+
}
71+
}
72+
73+
int main(void) {
74+
pfv old_handler = signal(SIGINT, handler);
75+
if (old_handler == SIG_ERR) {
76+
perror("SIGINT handler");
77+
/* Handle error */
78+
}
79+
80+
/* Main code loop */
81+
82+
return EXIT_SUCCESS;
83+
}
84+
85+
```
86+
87+
## Noncompliant Code Example (POSIX)
88+
89+
POSIX is less restrictive than C about what applications can do in signal handlers. It has a long list of [asynchronous-safe](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-asynchronous-safe) functions that can be called. (See [SIG30-C. Call only asynchronous-safe functions within signal handlers](https://wiki.sei.cmu.edu/confluence/display/c/SIG30-C.+Call+only+asynchronous-safe+functions+within+signal+handlers).) Many of these functions set `errno` on error, which can lead to a signal handler being executed between a call to a failed function and the subsequent inspection of `errno`. Consequently, the value inspected is not the one set by that function but the one set by a function call in the signal handler. POSIX applications can avoid this problem by ensuring that signal handlers containing code that might alter `errno`; always save the value of `errno` on entry and restore it before returning.
90+
91+
The signal handler in this noncompliant code example alters the value of `errno`. As a result, it can cause incorrect error handling if executed between a failed function call and the subsequent inspection of `errno`:
92+
93+
```cpp
94+
#include <signal.h>
95+
#include <stdlib.h>
96+
#include <errno.h>
97+
#include <sys/wait.h>
98+
99+
void reaper(int signum) {
100+
errno = 0;
101+
for (;;) {
102+
int rc = waitpid(-1, NULL, WNOHANG);
103+
if ((0 == rc) || (-1 == rc && EINTR != errno)) {
104+
break;
105+
}
106+
}
107+
if (ECHILD != errno) {
108+
/* Handle error */
109+
}
110+
}
111+
112+
int main(void) {
113+
struct sigaction act;
114+
act.sa_handler = reaper;
115+
act.sa_flags = 0;
116+
if (sigemptyset(&act.sa_mask) != 0) {
117+
/* Handle error */
118+
}
119+
if (sigaction(SIGCHLD, &act, NULL) != 0) {
120+
/* Handle error */
121+
}
122+
123+
/* ... */
124+
125+
return EXIT_SUCCESS;
126+
}
127+
128+
```
129+
130+
## Compliant Solution (POSIX)
131+
132+
This compliant solution saves and restores the value of `errno` in the signal handler:
133+
134+
```cpp
135+
#include <signal.h>
136+
#include <stdlib.h>
137+
#include <errno.h>
138+
#include <sys/wait.h>
139+
140+
void reaper(int signum) {
141+
errno_t save_errno = errno;
142+
errno = 0;
143+
for (;;) {
144+
int rc = waitpid(-1, NULL, WNOHANG);
145+
if ((0 == rc) || (-1 == rc && EINTR != errno)) {
146+
break;
147+
}
148+
}
149+
if (ECHILD != errno) {
150+
/* Handle error */
151+
}
152+
errno = save_errno;
153+
}
154+
155+
int main(void) {
156+
struct sigaction act;
157+
act.sa_handler = reaper;
158+
act.sa_flags = 0;
159+
if (sigemptyset(&act.sa_mask) != 0) {
160+
/* Handle error */
161+
}
162+
if (sigaction(SIGCHLD, &act, NULL) != 0) {
163+
/* Handle error */
164+
}
165+
166+
/* ... */
167+
168+
return EXIT_SUCCESS;
169+
}
170+
171+
```
172+
173+
## Risk Assessment
174+
175+
Referencing indeterminate values of `errno` is [undefined behavior](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-undefinedbehavior).
176+
177+
<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> ERR32-C </td> <td> Low </td> <td> Unlikely </td> <td> Low </td> <td> <strong>P3</strong> </td> <td> <strong>L3</strong> </td> </tr> </tbody> </table>
178+
179+
180+
## Automated Detection
181+
182+
<table> <tbody> <tr> <th> Tool </th> <th> Version </th> <th> Checker </th> <th> Description </th> </tr> <tr> <td> <a> Axivion Bauhaus Suite </a> </td> <td> 7.2.0 </td> <td> <strong>CertC-ERR32</strong> </td> <td> </td> </tr> <tr> <td> <a> Compass/ROSE </a> </td> <td> </td> <td> </td> <td> Could detect violations of this rule by looking for signal handlers that themselves call <code>signal()</code> . A violation is reported if the call fails and the handler therefore checks <code>errno</code> . A violation also exists if the signal handler modifies <code>errno</code> without first copying its value elsewhere </td> </tr> <tr> <td> <a> Coverity </a> </td> <td> 2017.07 </td> <td> <strong>MISRA C 2012 Rule 22.8</strong> <strong><strong>MISRA C 2012 Rule 22.9</strong></strong> <strong><strong><strong>MISRA C 2012 Rule 22.10</strong></strong></strong> </td> <td> Implemented </td> </tr> <tr> <td> <a> Helix QAC </a> </td> <td> 2022.3 </td> <td> <strong>C2031, C4781, C4782, C4783</strong> <strong>C++4781, C++4782, C++4783</strong> </td> <td> </td> </tr> <tr> <td> <a> Klocwork </a> </td> <td> 2022.3 </td> <td> <strong>MISRA.INCL.SIGNAL.2012</strong> <strong>MISRA.STDLIB.SIGNAL</strong> </td> <td> </td> </tr> <tr> <td> <a> LDRA tool suite </a> </td> <td> 9.7.1 </td> <td> 44 S </td> <td> Enhanced enforcement </td> </tr> <tr> <td> <a> Parasoft C/C++test </a> </td> <td> 2022.1 </td> <td> <strong>CERT_C-ERR32-a</strong> </td> <td> Properly use errno value </td> </tr> <tr> <td> <a> Polyspace Bug Finder </a> </td> <td> R2022b </td> <td> <a> CERT C: Rule ERR32-C </a> </td> <td> Checks for misuse of errno in a signal handler (rule fully covered) </td> </tr> <tr> <td> <a> PRQA QA-C </a> </td> <td> 9.7 </td> <td> <strong>2031</strong> </td> <td> </td> </tr> </tbody> </table>
183+
184+
185+
## Related Vulnerabilities
186+
187+
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+ERR32-C).
188+
189+
## Related Guidelines
190+
191+
[Key here](https://wiki.sei.cmu.edu/confluence/display/c/How+this+Coding+Standard+is+Organized#HowthisCodingStandardisOrganized-RelatedGuidelines) (explains table format and definitions)
192+
193+
<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> SIG30-C. Call only asynchronous-safe functions within signal handlers </a> </td> <td> Prior to 2018-01-12: CERT: Unspecified Relationship </td> </tr> <tr> <td> <a> CERT C Secure Coding Standard </a> </td> <td> <a> SIG31-C. Do not access shared objects in signal handlers </a> </td> <td> Prior to 2018-01-12: CERT: Unspecified Relationship </td> </tr> </tbody> </table>
194+
195+
196+
## Bibliography
197+
198+
<table> <tbody> <tr> <td> \[ <a> ISO/IEC 9899:2011 </a> \] </td> <td> Subclause 7.14.1.1, "The <code>signal</code> Function" </td> </tr> </tbody> </table>
199+
200+
201+
## Implementation notes
202+
203+
The rule is enforced in the context of a single function.
204+
205+
## References
206+
207+
* CERT-C: [ERR32-C: Do not rely on indeterminate values of errno](https://wiki.sei.cmu.edu/confluence/display/c)

‎c/cert/src/rules/ERR32-C/DoNotRelyOnIndeterminateValuesOfErrno.ql

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
/**
2+
* @id c/cert/do-not-rely-on-indeterminate-values-of-errno
3+
* @name ERR32-C: Do not rely on indeterminate values of errno
4+
* @description Do not rely on indeterminate values of errno. This may result in undefined behavior.
5+
* @kind problem
6+
* @precision high
7+
* @problem.severity error
8+
* @tags external/cert/id/err32-c
9+
* correctness
10+
* external/cert/obligation/rule
11+
*/
12+
13+
import cpp
14+
import codingstandards.c.cert
15+
import codingstandards.c.Errno
16+
import semmle.code.cpp.controlflow.Guards
17+
18+
/**
19+
* A call to function `signal`
20+
*/
21+
class SignalCall extends FunctionCall {
22+
SignalCall() { this.getTarget().hasGlobalName("signal") }
23+
}
24+
25+
/**
26+
* A call to `abort` or `_Exit`
27+
*/
28+
class AbortCall extends FunctionCall {
29+
AbortCall() { this.getTarget().hasGlobalName(["abort", "_Exit"]) }
30+
}
31+
32+
/**
33+
* A check on `signal` call return value
34+
* `if (signal(SIGINT, handler) == SIG_ERR)`
35+
*/
36+
class SignalCheckOperation extends EqualityOperation, GuardCondition {
37+
BasicBlock errorSuccessor;
38+
39+
SignalCheckOperation() {
40+
this.getAnOperand() = any(MacroInvocation m | m.getMacroName() = "SIG_ERR").getExpr() and
41+
(
42+
this.getOperator() = "==" and
43+
this.controls(errorSuccessor, true)
44+
or
45+
this.getOperator() = "!=" and
46+
this.controls(errorSuccessor, false)
47+
)
48+
}
49+
50+
BasicBlock getCheckedSuccessor() {
51+
result != errorSuccessor and result = this.getASuccessor()
52+
}
53+
54+
BasicBlock getErrorSuccessor() { result = errorSuccessor }
55+
}
56+
57+
/**
58+
* Models signal handlers that call signal() and return
59+
*/
60+
class SignalCallingHandler extends Function {
61+
SignalCall registration;
62+
63+
SignalCallingHandler() {
64+
// is a signal handler
65+
this = registration.getArgument(1).(FunctionAccess).getTarget() and
66+
// calls signal() on the handled signal
67+
exists(SignalCall sCall |
68+
sCall.getEnclosingFunction() = this and
69+
DataFlow::localFlow(DataFlow::parameterNode(this.getParameter(0)),
70+
DataFlow::exprNode(sCall.getArgument(0))) and
71+
// does not abort on error
72+
not exists(SignalCheckOperation sCheck, AbortCall abort |
73+
DataFlow::localExprFlow(sCall, sCheck.getAnOperand()) and
74+
abort = sCheck.getErrorSuccessor().(BlockStmt).getStmt(0).(ExprStmt).getExpr()
75+
)
76+
)
77+
}
78+
79+
SignalCall getCall() { result = registration }
80+
}
81+
82+
/**
83+
* CFG nodes preceeding `ErrnoRead`
84+
*/
85+
ControlFlowNode preceedErrnoRead(ErrnoRead er) {
86+
result = er
87+
or
88+
exists(ControlFlowNode mid |
89+
result = mid.getAPredecessor() and
90+
mid = preceedErrnoRead(er) and
91+
// stop recursion on calls to `abort` and `_Exit`
92+
not result instanceof AbortCall and
93+
// stop recursion on successful `SignalCheckOperation`
94+
not result = any(SignalCheckOperation o).getCheckedSuccessor()
95+
)
96+
}
97+
98+
from ErrnoRead errno, SignalCall signal
99+
where
100+
not isExcluded(errno, Contracts5Package::doNotRelyOnIndeterminateValuesOfErrnoQuery()) and
101+
exists(SignalCallingHandler handler |
102+
// errno read after the handler returns
103+
handler.getCall() = signal
104+
or
105+
// errno read inside the handler
106+
signal.getEnclosingFunction() = handler
107+
|
108+
signal = preceedErrnoRead(errno)
109+
)
110+
select errno, "`errno` has indeterminate value after this $@.", signal, signal.toString()

‎c/cert/src/rules/ERR33-C/DetectAndHandleStandardLibraryErrors.md

Lines changed: 379 additions & 0 deletions
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)