Skip to content

Commit 0cf71fd

Browse files
GrosQuilduGrosQuildu
committed
add data flow
1 parent 3dccc29 commit 0cf71fd

File tree

4 files changed

+114
-60
lines changed

4 files changed

+114
-60
lines changed

‎cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.ql‎

Lines changed: 77 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -10,55 +10,76 @@
1010
* @group security
1111
*/
1212

13-
import cpp
13+
import cpp
14+
import semmle.code.cpp.dataflow.new.DataFlow
1415

1516

16-
/* List from https://man7.org/linux/man-pages/man3/stdio.3.html */
17-
class StdioFunction extends Function {
18-
StdioFunction() {
19-
this.getName() in [
20-
"fopen", "freopen", "fflush", "fclose", "fread", "fwrite", "fgetc", "fgets", "fputc",
21-
"fputs", "getc", "getchar", "gets", "putc", "putchar", "puts", "ungetc", "fprintf",
22-
"fscanf", "printf", "scanf", "sprintf", "sscanf", "vprintf", "vfprintf", "vsprintf",
23-
"fgetpos", "fsetpos", "ftell", "fseek", "rewind", "clearerr", "feof", "ferror", "perror",
24-
"setbuf", "setvbuf", "remove", "rename", "tmpfile", "tmpnam",
25-
]
26-
}
27-
}
28-
29-
/* List from https://man7.org/linux/man-pages/man3/syslog.3.html */
30-
class SyslogFunction extends Function {
31-
SyslogFunction() { this.getName() in ["syslog", "vsyslog",] }
32-
}
33-
34-
/* List from https://man7.org/linux/man-pages/man0/stdlib.h.0p.html */
35-
class StdlibFunction extends Function {
36-
StdlibFunction() { this.getName() in ["malloc", "calloc", "realloc", "free",] }
37-
}
38-
39-
predicate isAsyncUnsafe(Function signalHandler) {
40-
exists(Function f |
41-
signalHandler.calls+(f) and
42-
(
43-
f instanceof StdioFunction or
44-
f instanceof SyslogFunction or
45-
f instanceof StdlibFunction
46-
)
47-
)
48-
}
17+
/* List from https://man7.org/linux/man-pages/man3/stdio.3.html */
18+
class StdioFunction extends Function {
19+
StdioFunction() {
20+
this.getName() in [
21+
"fopen", "freopen", "fflush", "fclose", "fread", "fwrite", "fgetc", "fgets", "fputc",
22+
"fputs", "getc", "getchar", "gets", "putc", "putchar", "puts", "ungetc", "fprintf",
23+
"fscanf", "printf", "scanf", "sprintf", "sscanf", "vprintf", "vfprintf", "vsprintf",
24+
"fgetpos", "fsetpos", "ftell", "fseek", "rewind", "clearerr", "feof", "ferror", "perror",
25+
"setbuf", "setvbuf", "remove", "rename", "tmpfile", "tmpnam",
26+
]
27+
}
28+
}
29+
30+
/* List from https://man7.org/linux/man-pages/man3/syslog.3.html */
31+
class SyslogFunction extends Function {
32+
SyslogFunction() { this.getName() in ["syslog", "vsyslog",] }
33+
}
34+
35+
/* List from https://man7.org/linux/man-pages/man0/stdlib.h.0p.html */
36+
class StdlibFunction extends Function {
37+
StdlibFunction() { this.getName() in ["malloc", "calloc", "realloc", "free",] }
38+
}
39+
40+
predicate isAsyncUnsafe(Function signalHandler) {
41+
exists(Function f |
42+
signalHandler.calls+(f) and
43+
(
44+
f instanceof StdioFunction or
45+
f instanceof SyslogFunction or
46+
f instanceof StdlibFunction
47+
)
48+
)
49+
}
50+
51+
/**
52+
* Flows from any function ptr
53+
*/
54+
module HandlerToSignalConfiguration implements DataFlow::ConfigSig {
55+
predicate isSource(DataFlow::Node source) {
56+
source.asExpr() = any(Function f || f.getAnAccess())
57+
}
58+
59+
predicate isSink(DataFlow::Node sink) {
60+
sink = sink
61+
}
62+
}
63+
module HandlerToSignal = DataFlow::Global<HandlerToSignalConfiguration>;
4964

50-
// signal(SIGX, signalHandler)
65+
/**
66+
* signal(SIGX, signalHandler)
67+
*/
5168
predicate isSignal(FunctionCall signalCall, Function signalHandler) {
5269
signalCall.getTarget().getName() = "signal"
53-
and signalHandler.getAnAccess() = signalCall.getArgument(1).getAChild*()
70+
and exists(DataFlow::Node source, DataFlow::Node sink |
71+
HandlerToSignal::flow(source, sink)
72+
and source.asExpr() = signalHandler.getAnAccess()
73+
and sink.asExpr() = signalCall.getArgument(1)
74+
)
5475
}
5576

5677
/**
5778
* struct sigaction sigactVar = ...
5879
* sigaction(SIGX, &sigactVar, ...)
5980
*/
6081
predicate isSigaction(FunctionCall sigactionCall, Function signalHandler, Variable sigactVar) {
61-
exists(Struct sigactStruct, Field handlerField |
82+
exists(Struct sigactStruct, Field handlerField, DataFlow::Node source, DataFlow::Node sink |
6283
sigactionCall.getTarget().getName() = "sigaction"
6384
and sigactionCall.getArgument(1).getAChild*() = sigactVar.getAnAccess()
6485

@@ -70,27 +91,36 @@ predicate isSigaction(FunctionCall sigactionCall, Function signalHandler, Variab
7091
or
7192
exists(Union u | u.getAField() = handlerField and u = sigactStruct.getAField().getType())
7293
)
94+
7395
and (
74-
exists(Assignment a, ValueFieldAccess dfa |
75-
// sigactVar.sa_handler = &signalHandler
76-
a.getLValue() = dfa.getAChild*()
77-
and a.getRValue().getAChild*() = signalHandler.getAnAccess()
78-
and dfa.getTarget() = handlerField
79-
and dfa.getQualifier+() = sigactVar.getAnAccess()
96+
// sigactVar.sa_handler = &signalHandler
97+
exists(Assignment a, ValueFieldAccess vfa |
98+
vfa.getTarget() = handlerField
99+
and vfa.getQualifier+() = sigactVar.getAnAccess()
100+
and a.getLValue() = vfa.getAChild*()
101+
102+
and source.asExpr() = signalHandler.getAnAccess()
103+
and sink.asExpr() = a.getRValue()
104+
and HandlerToSignal::flow(source, sink)
80105
)
81106
or
107+
// struct sigaction sigactVar = {.sa_sigaction = signalHandler};
82108
exists(ClassAggregateLiteral initLit |
83-
// struct sigaction sigactVar = {.sa_sigaction = signalHandler};
84-
// varDec.getVariable() = sigactVar
85109
sigactVar.getInitializer().getExpr() = initLit
86-
and signalHandler.getAnAccess() = initLit.getAFieldExpr(handlerField).getAChild*()
110+
and source.asExpr() = signalHandler.getAnAccess()
111+
and sink.asExpr() = initLit.getAFieldExpr(handlerField).getAChild*()
112+
and HandlerToSignal::flow(source, sink)
87113
)
88114
)
89115
)
90116
}
91117

118+
/**
119+
* Determine if new signals are blocked
120+
* TODO: should only find writes and only for correct (or all) signals
121+
* TODO: should detect other block mechanisms
122+
*/
92123
predicate isSignalDeliveryBlocked(Variable sigactVar) {
93-
// TODO: should only find writes and for specific signals
94124
exists(ValueFieldAccess dfa |
95125
dfa.getQualifier+() = sigactVar.getAnAccess() and dfa.getTarget().getName() = "sa_mask"
96126
)

‎cpp/test/include/libc/signal.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@
33
#ifndef HEADER_SIGNAL_STUB_H
44
#define HEADER_SIGNAL_STUB_H
55

6-
7-
86
#define SIGALRM 14
97
#define SIGSEGV 11
108
#define SIGTERM 15
@@ -13,8 +11,8 @@
1311
#define SA_SIGINFO 4
1412

1513
{} // to silent error from codeql's extractor
16-
typedef void (*sighandler_t)(int);
17-
extern int signal(int, sighandler_t);
14+
typedef void (*sig_t)(int);
15+
extern int signal(int, sig_t);
1816

1917
typedef struct {
2018
unsigned long sig[64];

‎cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.c‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ void safe_handler(int sig) {
2323
}
2424
}
2525

26+
void safe_handler2(int signo, siginfo_t *info, void *context) {
27+
if (signo == SIGALRM) {
28+
kill(1, SIGALRM);
29+
}
30+
}
31+
2632
void unsafe_handler(int sig) {
2733
transitive_call();
2834
}
@@ -34,7 +40,7 @@ void unsafe_handler2(int signo, siginfo_t *info, void *context) {
3440
transitive_call2();
3541
}
3642

37-
void unsafe_handler3(int signal) {
43+
void unsafe_handler3(int signo, siginfo_t *info, void *context) {
3844
transitive_call3();
3945
}
4046

@@ -74,7 +80,7 @@ static void df_handler(int sig) {
7480
sigdie("some log %d", sig);
7581
}
7682

77-
sighandler_t register_signal(int signum, sighandler_t handler) {
83+
sig_t register_signal(int signum, sig_t handler) {
7884
struct sigaction sa, osa;
7985
memset(&sa, 0, sizeof(sa));
8086
sa.sa_handler = handler;
@@ -92,6 +98,15 @@ int main() {
9298
_Exit(EXIT_FAILURE);
9399
}
94100

101+
// Safe handler 2
102+
struct sigaction actG2 = {0};
103+
actG2.sa_flags = SA_SIGINFO;
104+
actG2.sa_sigaction = &safe_handler2;
105+
if (sigaction(SIGSEGV, &actG2, NULL) == -1) {
106+
perror("sigaction");
107+
_Exit(EXIT_FAILURE);
108+
}
109+
95110
// Unsafe example 1
96111
if (signal(SIGALRM, unsafe_handler) == SIG_ERR) {
97112
perror("Unable to catch SIGALRM");
@@ -127,7 +142,7 @@ int main() {
127142
}
128143

129144
// Unsafe example 5
130-
struct sigaction act4 = {.sa_flags = SA_SIGINFO, .sa_sigaction = unsafe_handler2};
145+
struct sigaction act4 = {.sa_flags = SA_SIGINFO, .sa_sigaction = unsafe_handler3};
131146
act4.sa_mask = sigset;
132147
if (sigaction(SIGSEGV, &act4, NULL) == -1) {
133148
perror("sigaction 4");
@@ -137,13 +152,20 @@ int main() {
137152
// Unsafe example 6
138153
struct sigaction act5 = {.sa_mask = sigset};
139154
act5.sa_flags = SA_SIGINFO;
140-
act5.sa_sigaction = &unsafe_handler2;
155+
act5.sa_sigaction = &unsafe_handler3;
141156
if (sigaction(SIGSEGV, &act5, NULL) == -1) {
142157
perror("sigaction 5");
143158
_Exit(EXIT_FAILURE);
144159
}
145160

146-
// Unsafe example 7, indirect handler registration
161+
// Unsafe example 7
162+
sig_t wrapper = unsafe_handler;
163+
if (signal(SIGALRM, wrapper) == SIG_ERR) {
164+
perror("Unable to catch SIGALRM");
165+
_Exit(EXIT_FAILURE);
166+
}
167+
168+
// Unsafe example 8, indirect handler registration
147169
if(register_signal(SIGALRM, df_handler)) {
148170
_exit(EXIT_FAILURE);
149171
}

‎cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.expected‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
| AsyncUnsafeSignalHandler.c:24:6:24:19 | unsafe_handler | is a non-trivial signal handler that may be using not async-safe functions. Registered by $@ | AsyncUnsafeSignalHandler.c:47:9:47:14 | call to signal | call to signal |
2-
| AsyncUnsafeSignalHandler.c:28:6:28:20 | unsafe_handler2 | is a non-trivial signal handler that may be using not async-safe functions. Registered by $@ | AsyncUnsafeSignalHandler.c:57:9:57:17 | call to sigaction | call to sigaction |
3-
| AsyncUnsafeSignalHandler.c:28:6:28:20 | unsafe_handler2 | is a non-trivial signal handler that may be using not async-safe functions. Registered by $@ | AsyncUnsafeSignalHandler.c:64:9:64:17 | call to sigaction | call to sigaction |
4-
| AsyncUnsafeSignalHandler.c:28:6:28:20 | unsafe_handler2 | is a non-trivial signal handler that may be using not async-safe functions. Registered by $@ | AsyncUnsafeSignalHandler.c:91:9:91:17 | call to sigaction | call to sigaction |
1+
| AsyncUnsafeSignalHandler.c:32:6:32:19 | unsafe_handler | is a non-trivial signal handler that uses not async-safe functions. Delivery of new signals may be not blocked when the handler executes. Handler is registered by $@ | AsyncUnsafeSignalHandler.c:111:9:111:14 | call to signal | call to signal |
2+
| AsyncUnsafeSignalHandler.c:32:6:32:19 | unsafe_handler | is a non-trivial signal handler that uses not async-safe functions. Delivery of new signals may be not blocked when the handler executes. Handler is registered by $@ | AsyncUnsafeSignalHandler.c:163:9:163:14 | call to signal | call to signal |
3+
| AsyncUnsafeSignalHandler.c:36:6:36:20 | unsafe_handler2 | is a non-trivial signal handler that uses not async-safe functions. Delivery of new signals may be not blocked when the handler executes. Handler is registered by $@ | AsyncUnsafeSignalHandler.c:121:9:121:17 | call to sigaction | call to sigaction |
4+
| AsyncUnsafeSignalHandler.c:36:6:36:20 | unsafe_handler2 | is a non-trivial signal handler that uses not async-safe functions. Delivery of new signals may be not blocked when the handler executes. Handler is registered by $@ | AsyncUnsafeSignalHandler.c:128:9:128:17 | call to sigaction | call to sigaction |
5+
| AsyncUnsafeSignalHandler.c:36:6:36:20 | unsafe_handler2 | is a non-trivial signal handler that uses not async-safe functions. Handler is registered by $@ | AsyncUnsafeSignalHandler.c:139:9:139:17 | call to sigaction | call to sigaction |
6+
| AsyncUnsafeSignalHandler.c:43:6:43:20 | unsafe_handler3 | is a non-trivial signal handler that uses not async-safe functions. Handler is registered by $@ | AsyncUnsafeSignalHandler.c:147:9:147:17 | call to sigaction | call to sigaction |
7+
| AsyncUnsafeSignalHandler.c:43:6:43:20 | unsafe_handler3 | is a non-trivial signal handler that uses not async-safe functions. Handler is registered by $@ | AsyncUnsafeSignalHandler.c:156:9:156:17 | call to sigaction | call to sigaction |
8+
| AsyncUnsafeSignalHandler.c:76:13:76:22 | df_handler | is a non-trivial signal handler that uses not async-safe functions. Handler is registered by $@ | AsyncUnsafeSignalHandler.c:88:6:88:14 | call to sigaction | call to sigaction |

0 commit comments

Comments
 (0)