Skip to content

Commit c38fe8c

Browse files
mbaludambaluda
committed
Rule ERR33-C
1 parent f9a2c5e commit c38fe8c

File tree

9 files changed

+821
-2
lines changed

9 files changed

+821
-2
lines changed

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

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

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

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
/**
2+
* @id c/cert/detect-and-handle-standard-library-errors
3+
* @name ERR33-C: Detect and handle standard library errors
4+
* @description Detect and handle standard library errors.
5+
* @kind problem
6+
* @precision high
7+
* @problem.severity error
8+
* @tags external/cert/id/err33-c
9+
* correctness
10+
* external/cert/obligation/rule
11+
*/
12+
13+
import cpp
14+
import codingstandards.c.cert
15+
import semmle.code.cpp.commons.NULL
16+
import codingstandards.cpp.ReadErrorsAndEOF
17+
import semmle.code.cpp.dataflow.DataFlow
18+
19+
/**
20+
* Classifies error returning function calls based on the
21+
* type and value of the required checked
22+
*/
23+
class ExpectedErrReturn extends FunctionCall {
24+
Expr errValue;
25+
string errOperator;
26+
27+
ExpectedErrReturn() {
28+
errOperator = ["==", "!="] and
29+
(
30+
errValue.(Literal).getValue() = "0" and
31+
this.getTarget()
32+
.hasName([
33+
"asctime_s", "at_quick_exit", "atexit", "ctime_s", "fgetpos", "fopen_s", "freopen_s",
34+
"fseek", "fsetpos", "mbsrtowcs_s", "mbstowcs_s", "raise", "remove", "rename",
35+
"setvbuf", "strerror_s", "strftime", "strtod", "strtof", "strtold", "timespec_get",
36+
"tmpfile_s", "tmpnam_s", "tss_get", "wcsftime", "wcsrtombs_s", "wcstod", "wcstof",
37+
"wcstold", "wcstombs_s", "wctrans", "wctype"
38+
])
39+
or
40+
errValue instanceof NULL and
41+
this.getTarget()
42+
.hasName([
43+
"aligned_alloc", "bsearch_s", "bsearch", "calloc", "fgets", "fopen", "freopen",
44+
"getenv_s", "getenv", "gets_s", "gmtime_s", "gmtime", "localtime_s", "localtime",
45+
"malloc", "memchr", "realloc", "setlocale", "strchr", "strpbrk", "strrchr", "strstr",
46+
"strtok_s", "strtok", "tmpfile", "tmpnam", "wcschr", "wcspbrk", "wcsrchr", "wcsstr",
47+
"wcstok_s", "wcstok", "wmemchr"
48+
])
49+
or
50+
errValue = any(EOFInvocation i).getExpr() and
51+
this.getTarget()
52+
.hasName([
53+
"fclose", "fflush", "fputs", "fputws", "fscanf_s", "fscanf", "fwscanf_s", "fwscanf",
54+
"scanf_s", "scanf", "sscanf_s", "sscanf", "swscanf_s", "swscanf", "ungetc",
55+
"vfscanf_s", "vfscanf", "vfwscanf_s", "vfwscanf", "vscanf_s", "vscanf", "vsscanf_s",
56+
"vsscanf", "vswscanf_s", "vswscanf", "vwscanf_s", "vwscanf", "wctob", "wscanf_s",
57+
"wscanf", "fgetc", "fputc", "getc", "getchar", "putc", "putchar", "puts"
58+
])
59+
or
60+
errValue = any(WEOFInvocation i).getExpr() and
61+
this.getTarget()
62+
.hasName([
63+
"btowc", "fgetwc", "fputwc", "getwc", "getwchar", "putwc", "ungetwc", "putwchar"
64+
])
65+
or
66+
errValue = any(EnumConstantAccess i | i.toString() = "thrd_error") and
67+
this.getTarget()
68+
.hasName([
69+
"cnd_broadcast", "cnd_init", "cnd_signal", "cnd_timedwait", "cnd_wait", "mtx_init",
70+
"mtx_lock", "mtx_timedlock", "mtx_trylock", "mtx_unlock", "thrd_create",
71+
"thrd_detach", "thrd_join", "tss_create", "tss_set"
72+
])
73+
or
74+
errValue = any(EnumConstantAccess i | i.toString() = "thrd_nomem") and
75+
this.getTarget().hasName(["cnd_init", "thrd_create"])
76+
or
77+
errValue = any(EnumConstantAccess i | i.toString() = "thrd_timedout") and
78+
this.getTarget().hasName(["cnd_timedwait", "mtx_timedlock"])
79+
or
80+
errValue = any(EnumConstantAccess i | i.toString() = "thrd_busy") and
81+
this.getTarget().hasName(["mtx_trylock"])
82+
or
83+
errValue = any(MacroInvocation i | i.getMacroName() = "UINTMAX_MAX").getExpr() and
84+
this.getTarget().hasName(["strtoumax", "wcstoumax"])
85+
or
86+
errValue = any(MacroInvocation i | i.getMacroName() = "ULONG_MAX").getExpr() and
87+
this.getTarget().hasName(["strtoul", "wcstoul"])
88+
or
89+
errValue = any(MacroInvocation i | i.getMacroName() = "ULLONG_MAX").getExpr() and
90+
this.getTarget().hasName(["strtoull", "wcstoull"])
91+
or
92+
errValue = any(MacroInvocation i | i.getMacroName() = "SIG_ERR").getExpr() and
93+
this.getTarget().hasName(["signal"])
94+
or
95+
errValue = any(MacroInvocation i | i.getMacroName() = ["INTMAX_MAX", "INTMAX_MIN"]).getExpr() and
96+
this.getTarget().hasName(["strtoimax", "wcstoimax"])
97+
or
98+
errValue = any(MacroInvocation i | i.getMacroName() = ["LONG_MAX", "LONG_MIN"]).getExpr() and
99+
this.getTarget().hasName(["strtol", "wcstol"])
100+
or
101+
errValue = any(MacroInvocation i | i.getMacroName() = ["LLONG_MAX", "LLONG_MIN"]).getExpr() and
102+
this.getTarget().hasName(["strtoll", "wcstoll"])
103+
or
104+
errValue.(UnaryMinusExpr).getOperand().(Literal).getValue() = "1" and
105+
this.getTarget()
106+
.hasName([
107+
"c16rtomb", "c32rtomb", "clock", "ftell", "mbrtoc16", "mbrtoc32", "mbsrtowcs",
108+
"mbstowcs", "mktime", "time", "wcrtomb", "wcsrtombs", "wcstombs"
109+
])
110+
or
111+
errValue.(UnaryMinusExpr).getOperand().(Literal).getValue() = "1" and
112+
not this.getArgument(0) instanceof NULL and
113+
this.getTarget().hasName(["mblen", "mbrlen", "mbrtowc", "mbtowc", "wctomb_s", "wctomb"])
114+
or
115+
errValue.getType() instanceof IntType and
116+
this.getTarget().hasName(["fread", "fwrite"])
117+
)
118+
or
119+
errOperator = ["<", ">="] and
120+
(
121+
errValue.(Literal).getValue() = "0" and
122+
this.getTarget()
123+
.hasName([
124+
"fprintf_s", "fprintf", "fwprintf_s", "fwprintf", "printf_s", "snprintf_s",
125+
"snprintf", "sprintf_s", "sprintf", "swprintf_s", "swprintf", "thrd_sleep",
126+
"vfprintf_s", "vfprintf", "vfwprintf_s", "vfwprintf", "vprintf_s", "vsnprintf_s",
127+
"vsnprintf", "vsprintf_s", "vsprintf", "vswprintf_s", "vswprintf", "vwprintf_s",
128+
"wprintf_s", "printf", "vprintf", "wprintf", "vwprintf"
129+
])
130+
or
131+
errValue.getType() instanceof IntType and
132+
this.getTarget().hasName(["strxfrm", "wcsxfrm"])
133+
)
134+
or
135+
errOperator = "NA" and
136+
(
137+
errValue = any(Expr e) and
138+
this.getTarget()
139+
.hasName([
140+
"kill_dependency", "memcpy", "wmemcpy", "memmove", "wmemmove", "strcpy", "wcscpy",
141+
"strncpy", "wcsncpy", "strcat", "wcscat", "strncat", "wcsncat", "memset", "wmemset"
142+
])
143+
)
144+
}
145+
146+
Expr getErrValue() { result = errValue }
147+
148+
string getErrOperator() { result = errOperator }
149+
}
150+
151+
// Nodes following a file write before a call to `ferror` is performed
152+
ControlFlowNode ferrorNotchecked(FileWriteFunctionCall write) {
153+
result = write
154+
or
155+
exists(ControlFlowNode mid |
156+
mid = ferrorNotchecked(write) and
157+
//do not traverse the short-circuited CFG edge
158+
not isShortCircuitedEdge(mid, result) and
159+
result = mid.getASuccessor() and
160+
//Stop recursion on call to ferror on the correct file
161+
not accessSameTarget(result.(FerrorCall).getArgument(0), write.getFileExpr())
162+
)
163+
}
164+
165+
from ExpectedErrReturn err
166+
where
167+
not isExcluded(err, Contracts5Package::detectAndHandleStandardLibraryErrorsQuery()) and
168+
// calls that must be verified using the return value
169+
not exists(ComparisonOperation op |
170+
DataFlow::localExprFlow(err, op.getAnOperand()) and
171+
(err.getErrOperator() != "NA" implies op.getOperator() = err.getErrOperator()) and
172+
op.getAnOperand() = err.getErrValue() and
173+
// special case for function `realloc` where the returned pointer
174+
// should not be invalidated
175+
not (
176+
err.getTarget().hasName("realloc") and
177+
op.getAnOperand().(VariableAccess).getTarget() =
178+
err.getArgument(0).(VariableAccess).getTarget()
179+
)
180+
) and
181+
// EXCEPTIONS
182+
(
183+
// calls that can be verified using ferror() && feof()
184+
err.getTarget().hasName(["fgetc", "fgetwc", "getc", "getchar"])
185+
implies
186+
missingFeofFerrorChecks(err)
187+
) and
188+
(
189+
// calls that can be verified using ferror()
190+
err.getTarget().hasName(["fputc", "putc"])
191+
implies
192+
err.getEnclosingFunction() = ferrorNotchecked(err)
193+
) and
194+
(
195+
// ERR33-C-EX1: calls that can be ignored when cast to `void`
196+
err.getTarget()
197+
.hasName([
198+
"putchar", "putwchar", "puts", "printf", "vprintf", "wprintf", "vwprintf",
199+
"kill_dependency", "memcpy", "wmemcpy", "memmove", "wmemmove", "strcpy", "wcscpy",
200+
"strncpy", "wcsncpy", "strcat", "wcscat", "strncat", "wcsncat", "memset", "wmemset"
201+
])
202+
implies
203+
not err.getExplicitlyConverted() instanceof VoidConversion
204+
)
205+
select err,
206+
"Missing error detection for the call to function `" + err.getTarget() +
207+
"`. Undetected failures can lead to unexpected or undefined behavior."

‎c/cert/test/rules/ERR33-C/DetectAndHandleStandardLibraryErrors.expected

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
| test.c:18:3:18:11 | call to setlocale | Missing error detection for the call to function `setlocale`. Undetected failures can lead to unexpected or undefined behavior. |
2+
| test.c:24:23:24:31 | call to setlocale | Missing error detection for the call to function `setlocale`. Undetected failures can lead to unexpected or undefined behavior. |
3+
| test.c:29:22:29:27 | call to calloc | Missing error detection for the call to function `calloc`. Undetected failures can lead to unexpected or undefined behavior. |
4+
| test.c:35:7:35:13 | call to realloc | Missing error detection for the call to function `realloc`. Undetected failures can lead to unexpected or undefined behavior. |
5+
| test.c:46:3:46:7 | call to fseek | Missing error detection for the call to function `fseek`. Undetected failures can lead to unexpected or undefined behavior. |
6+
| test.c:52:3:52:10 | call to snprintf | Missing error detection for the call to function `snprintf`. Undetected failures can lead to unexpected or undefined behavior. |
7+
| test.c:60:3:60:9 | call to putchar | Missing error detection for the call to function `putchar`. Undetected failures can lead to unexpected or undefined behavior. |
8+
| test.c:63:3:63:8 | call to printf | Missing error detection for the call to function `printf`. Undetected failures can lead to unexpected or undefined behavior. |
9+
| test.c:74:22:74:30 | call to localtime | Missing error detection for the call to function `localtime`. Undetected failures can lead to unexpected or undefined behavior. |
10+
| test.c:80:3:80:7 | call to mblen | Missing error detection for the call to function `mblen`. Undetected failures can lead to unexpected or undefined behavior. |
11+
| test.c:97:5:97:9 | call to fputc | Missing error detection for the call to function `fputc`. Undetected failures can lead to unexpected or undefined behavior. |
12+
| test.c:105:5:105:11 | call to getchar | Missing error detection for the call to function `getchar`. Undetected failures can lead to unexpected or undefined behavior. |

‎c/cert/test/rules/ERR33-C/DetectAndHandleStandardLibraryErrors.qlref

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

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

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
#include <inttypes.h>
2+
#include <limits.h>
3+
#include <locale.h>
4+
#include <signal.h>
5+
#include <stdio.h>
6+
#include <stdlib.h>
7+
#include <string.h>
8+
#include <threads.h>
9+
#include <time.h>
10+
#include <wchar.h>
11+
12+
void *p;
13+
typedef struct {
14+
char sig_desc[32];
15+
} signal_info;
16+
17+
void f1(size_t n) {
18+
setlocale(LC_CTYPE, "en_US.UTF-8"); // NON_COMPLIANT
19+
20+
const char *save1 = setlocale(LC_CTYPE, "en_US.UTF-8"); // COMPLIANT
21+
if (NULL == save1) {
22+
}
23+
24+
const char *save2 = setlocale(LC_CTYPE, "en_US.UTF-8"); // NON_COMPLIANT
25+
if (save1 == save2) {
26+
}
27+
28+
signal_info *start =
29+
(signal_info *)calloc(n, sizeof(signal_info)); // NON_COMPLIANT
30+
31+
start = (signal_info *)calloc(n, sizeof(signal_info)); // COMPLIANT
32+
if (start == NULL) {
33+
}
34+
35+
p = realloc(p, n); // NON_COMPLIANT
36+
if (p == NULL) {
37+
}
38+
39+
void *q;
40+
q = realloc(p, n); // COMPLIANT
41+
if (q == NULL) {
42+
}
43+
}
44+
45+
void f2(FILE *f, long o) {
46+
fseek(f, o, SEEK_SET); // NON_COMPLIANT
47+
48+
if (fseek(f, o, SEEK_SET) != 0) { // COMPLIANT
49+
}
50+
51+
char buf[40];
52+
snprintf(buf, sizeof(buf), ""); // NON_COMPLIANT
53+
54+
int n = snprintf(buf, sizeof(buf), ""); // COMPLIANT
55+
if (n < 0) {
56+
}
57+
}
58+
59+
void f3() {
60+
putchar('C'); // NON_COMPLIANT
61+
(void)putchar('C'); // COMPLIANT
62+
63+
printf(""); // NON_COMPLIANT
64+
(void)printf(""); // COMPLIANT
65+
}
66+
void signal_handler(int signal) {}
67+
void f4() {
68+
FILE *f;
69+
char a[10];
70+
char b[10];
71+
time_t time;
72+
if (fprintf(f, "") < 0) { // COMPLIANT
73+
}
74+
struct tm *local = localtime(&time); // NON_COMPLIANT
75+
if (strftime(b, 10, "", local) == 0) { // COMPLIANT
76+
}
77+
if (clock() == (clock_t)(-1)) { // COMPLIANT
78+
}
79+
mblen(NULL, 0); // COMPLIANT
80+
mblen(a, 0); // NON_COMPLIANT
81+
if (mblen(a, 0) == -1) { // COMPLIANT
82+
}
83+
if (ftell(f) == -1L) { // COMPLIANT
84+
}
85+
if (fread(b, 1, 1, f) == 32) { // COMPLIANT
86+
}
87+
if (fwrite("", 1, 1, f) == 32) { // COMPLIANT
88+
}
89+
if (wctob(0) == EOF) { // COMPLIANT
90+
}
91+
if (fputc(0, f) == EOF) { // COMPLIANT
92+
}
93+
do {
94+
fputc(0, f); // COMPLIANT
95+
} while (!ferror(f));
96+
do {
97+
fputc(0, f); // NON_COMPLIANT
98+
} while (!feof(f));
99+
if (fgetc(f) == EOF) { // COMPLIANT
100+
}
101+
do {
102+
getchar(); // COMPLIANT
103+
} while ((!feof(stdin) && !ferror(stdin)));
104+
do {
105+
getchar(); // NON_COMPLIANT
106+
} while (!feof(stdin));
107+
if (aligned_alloc(0, 0) == NULL) { // COMPLIANT
108+
}
109+
if (signal(SIGINT, signal_handler) == SIG_ERR) { // COMPLIANT
110+
}
111+
cnd_t q;
112+
if (cnd_broadcast(&q) == thrd_error) { // COMPLIANT
113+
}
114+
if (cnd_init(&q) == thrd_nomem) { // COMPLIANT
115+
}
116+
if (cnd_init(&q) == thrd_error) { // COMPLIANT
117+
}
118+
mtx_t mutex;
119+
struct timespec ts;
120+
if (cnd_timedwait(&q, &mutex, &ts) == thrd_timedout) { // COMPLIANT
121+
}
122+
if (cnd_timedwait(&q, &mutex, &ts) == thrd_error) { // COMPLIANT
123+
}
124+
char *endptr;
125+
if (strtoumax("", &endptr, 0) == UINTMAX_MAX) { // COMPLIANT
126+
}
127+
if (strtoull("", &endptr, 0) == ULLONG_MAX) { // COMPLIANT
128+
}
129+
if (strtoul("", &endptr, 0) == ULONG_MAX) { // COMPLIANT
130+
}
131+
if (btowc(0) == WEOF) { // COMPLIANT
132+
}
133+
if (fgetwc(f) == WEOF) { // COMPLIANT
134+
}
135+
if (strxfrm(a, b, 10) >= 32) { // COMPLIANT
136+
}
137+
}

0 commit comments

Comments
 (0)