Skip to content

Commit 993dd74

Browse files
mbaludambaluda
committed
Rule ERR32-C
1 parent 315f09b commit 993dd74

File tree

7 files changed

+322
-31
lines changed

7 files changed

+322
-31
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+
None
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: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
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+
17+
class SignalCall extends FunctionCall {
18+
SignalCall() { this.getTarget().hasGlobalName("signal") }
19+
}
20+
21+
/**
22+
* Models signal handlers that call signal()
23+
*/
24+
class SignalCallingHandler extends Function {
25+
SignalCall sh;
26+
27+
SignalCallingHandler() {
28+
// is a signal handler
29+
this = sh.getArgument(1).(FunctionAccess).getTarget() and
30+
// calls signal()
31+
this.calls*(any(SignalCall c).getTarget())
32+
}
33+
34+
SignalCall getHandler() { result = sh }
35+
}
36+
37+
from ErrnoRead errno, SignalCall h
38+
where
39+
not isExcluded(errno, Contracts5Package::doNotRelyOnIndeterminateValuesOfErrnoQuery()) and
40+
// errno read in the handler
41+
exists(SignalCallingHandler sc |
42+
sc.getHandler() = h and
43+
(
44+
sc.calls*(errno.getEnclosingFunction())
45+
or
46+
// errno is read after the handle
47+
errno.(ControlFlowNode).getAPredecessor+() = sc.getHandler()
48+
)
49+
)
50+
select errno, "`errno` has indeterminate value after this $@.", h, h.toString()

‎c/cert/test/rules/ERR32-C/DoNotRelyOnIndeterminateValuesOfErrno.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
| test.c:12:5:12:10 | call to perror | `errno` has indeterminate value after this $@. | test.c:32:21:32:26 | call to signal | call to signal |
2+
| test.c:26:3:26:8 | call to perror | `errno` has indeterminate value after this $@. | test.c:41:17:41:22 | call to signal | call to signal |
3+
| test.c:34:5:34:10 | call to perror | `errno` has indeterminate value after this $@. | test.c:32:21:32:26 | call to signal | call to signal |

‎c/cert/test/rules/ERR32-C/DoNotRelyOnIndeterminateValuesOfErrno.qlref

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

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
#include <errno.h>
2+
#include <signal.h>
3+
#include <stdio.h>
4+
#include <stdlib.h>
5+
#include <sys/wait.h>
6+
7+
typedef void (*pfv)(int);
8+
9+
void handler1(int signum) {
10+
pfv old_handler = signal(signum, SIG_DFL);
11+
if (old_handler == SIG_ERR) {
12+
perror(""); // NON_COMPLIANT
13+
}
14+
}
15+
16+
void handler2(int signum) {
17+
pfv old_handler = signal(signum, SIG_DFL);
18+
if (old_handler == SIG_ERR) {
19+
abort(); // COMPLIANT
20+
}
21+
}
22+
23+
void handler3(int signum) { pfv old_handler = signal(signum, SIG_DFL); }
24+
25+
pfv helper4(int signum) {
26+
perror(""); // NON_COMPLIANT
27+
return signal(signum, SIG_DFL);
28+
}
29+
void handler4(int signum) { pfv old_handler = helper4(signum); }
30+
31+
int main(void) {
32+
pfv old_handler = signal(SIGINT, handler1);
33+
if (old_handler == SIG_ERR) {
34+
perror(""); // NON_COMPLIANT
35+
}
36+
37+
old_handler = signal(SIGINT, handler2);
38+
39+
old_handler = signal(SIGINT, handler3);
40+
41+
old_handler = signal(SIGINT, handler4);
42+
}

‎rule_packages/c/Contracts5.json

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,34 +4,30 @@
44
"properties": {
55
"obligation": "rule"
66
},
7-
"queries": [
8-
{
9-
"description": "Do not rely on indeterminate values of errno.",
10-
"kind": "problem",
11-
"name": "Do not rely on indeterminate values of errno",
12-
"precision": "high",
13-
"severity": "error",
14-
"short_name": "DoNotRelyOnIndeterminateValuesOfErrno",
15-
"tags": ["correctness"]
16-
}
17-
],
7+
"queries": [{
8+
"description": "Do not rely on indeterminate values of errno. This may result in undefined behavior.",
9+
"kind": "problem",
10+
"name": "Do not rely on indeterminate values of errno",
11+
"precision": "high",
12+
"severity": "error",
13+
"short_name": "DoNotRelyOnIndeterminateValuesOfErrno",
14+
"tags": ["correctness"]
15+
}],
1816
"title": "Do not rely on indeterminate values of errno"
1917
},
2018
"ERR33-C": {
2119
"properties": {
2220
"obligation": "rule"
2321
},
24-
"queries": [
25-
{
26-
"description": "Detect and handle standard library errors. Undetected failures can lead to unexpected or undefined behavior.",
27-
"kind": "problem",
28-
"name": "Detect and handle standard library errors",
29-
"precision": "high",
30-
"severity": "error",
31-
"short_name": "DetectAndHandleStandardLibraryErrors",
32-
"tags": ["correctness"]
33-
}
34-
],
22+
"queries": [{
23+
"description": "Detect and handle standard library errors. Undetected failures can lead to unexpected or undefined behavior.",
24+
"kind": "problem",
25+
"name": "Detect and handle standard library errors",
26+
"precision": "high",
27+
"severity": "error",
28+
"short_name": "DetectAndHandleStandardLibraryErrors",
29+
"tags": ["correctness"]
30+
}],
3531
"title": "Detect and handle standard library errors"
3632
}
3733
}

0 commit comments

Comments
 (0)