From f4598eff5eea494584f46ff7d9b0108108829e8f Mon Sep 17 00:00:00 2001
From: GrosQuildu
Date: Tue, 2 Jul 2024 22:07:22 +0200
Subject: [PATCH 1/5] sa_mask FP reduced, better sigaction detection
---
.../AsyncUnsafeSignalHandler.ql | 129 ++++++++++++------
.../AsyncUnsafeSignalHandler.c | 35 ++++-
.../AsyncUnsafeSignalHandler.expected | 7 +-
3 files changed, 122 insertions(+), 49 deletions(-)
diff --git a/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.ql b/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.ql
index 994daa0..9003a41 100644
--- a/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.ql
+++ b/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.ql
@@ -10,56 +10,99 @@
* @group security
*/
-import cpp
+ import cpp
-/* List from https://man7.org/linux/man-pages/man3/stdio.3.html */
-class StdioFunction extends Function {
- StdioFunction() {
- this.getName() in [
- "fopen", "freopen", "fflush", "fclose", "fread", "fwrite", "fgetc", "fgets", "fputc",
- "fputs", "getc", "getchar", "gets", "putc", "putchar", "puts", "ungetc", "fprintf",
- "fscanf", "printf", "scanf", "sprintf", "sscanf", "vprintf", "vfprintf", "vsprintf",
- "fgetpos", "fsetpos", "ftell", "fseek", "rewind", "clearerr", "feof", "ferror", "perror",
- "setbuf", "setvbuf", "remove", "rename", "tmpfile", "tmpnam",
- ]
- }
-}
-/* List from https://man7.org/linux/man-pages/man3/syslog.3.html */
-class SyslogFunction extends Function {
- SyslogFunction() { this.getName() in ["syslog", "vsyslog",] }
-}
+ /* List from https://man7.org/linux/man-pages/man3/stdio.3.html */
+ class StdioFunction extends Function {
+ StdioFunction() {
+ this.getName() in [
+ "fopen", "freopen", "fflush", "fclose", "fread", "fwrite", "fgetc", "fgets", "fputc",
+ "fputs", "getc", "getchar", "gets", "putc", "putchar", "puts", "ungetc", "fprintf",
+ "fscanf", "printf", "scanf", "sprintf", "sscanf", "vprintf", "vfprintf", "vsprintf",
+ "fgetpos", "fsetpos", "ftell", "fseek", "rewind", "clearerr", "feof", "ferror", "perror",
+ "setbuf", "setvbuf", "remove", "rename", "tmpfile", "tmpnam",
+ ]
+ }
+ }
+
+ /* List from https://man7.org/linux/man-pages/man3/syslog.3.html */
+ class SyslogFunction extends Function {
+ SyslogFunction() { this.getName() in ["syslog", "vsyslog",] }
+ }
+
+ /* List from https://man7.org/linux/man-pages/man0/stdlib.h.0p.html */
+ class StdlibFunction extends Function {
+ StdlibFunction() { this.getName() in ["malloc", "calloc", "realloc", "free",] }
+ }
+
+ predicate isAsyncUnsafe(Function signalHandler) {
+ exists(Function f |
+ signalHandler.calls+(f) and
+ (
+ f instanceof StdioFunction or
+ f instanceof SyslogFunction or
+ f instanceof StdlibFunction
+ )
+ )
+ }
-/* List from https://man7.org/linux/man-pages/man0/stdlib.h.0p.html */
-class StdlibFunction extends Function {
- StdlibFunction() { this.getName() in ["malloc", "calloc", "realloc", "free",] }
+// signal(SIGX, signalHandler)
+predicate isSignal(FunctionCall signalCall, Function signalHandler) {
+ signalCall.getTarget().getName() = "signal"
+ and signalHandler.getAnAccess() = signalCall.getArgument(1).getAChild*()
}
-predicate isAsyncUnsafe(Function signalHandler) {
- exists(Function f |
- signalHandler.calls+(f) and
- (
- f instanceof StdioFunction or
- f instanceof SyslogFunction or
- f instanceof StdlibFunction
+/**
+ * struct sigaction sigactVar = ...
+ * sigaction(SIGX, &sigactVar, ...)
+ */
+predicate isSigaction(FunctionCall sigactionCall, Function signalHandler) {
+ exists(Variable sigactVar, Struct sigactStruct, Field handlerField |
+ sigactionCall.getTarget().getName() = "sigaction"
+ and sigactionCall.getArgument(1).getAChild*() = sigactVar.getAnAccess()
+
+ // struct sigaction with the handler func
+ and sigactStruct.getName() = "sigaction"
+ and handlerField.getName() = ["sa_handler", "sa_sigaction", "__sa_handler", "__sa_sigaction", "__sigaction_u"]
+ and (
+ handlerField = sigactStruct.getAField()
+ or
+ exists(Union u | u.getAField() = handlerField and u = sigactStruct.getAField().getType())
+ )
+ and (
+ exists(Assignment a, ValueFieldAccess dfa |
+ // sigactVar.sa_handler = &signalHandler
+ a.getLValue() = dfa.getAChild*()
+ and a.getRValue().getAChild*() = signalHandler.getAnAccess()
+ and dfa.getTarget() = handlerField
+ and dfa.getQualifier+() = sigactVar.getAnAccess()
+ )
+ or
+ exists(VariableDeclarationEntry varDec, ClassAggregateLiteral init |
+ // struct sigaction sigactVar = {.sa_sigaction = signalHandler};
+ varDec.getVariable() = sigactVar
+ and sigactVar.getInitializer().getExpr() = init
+ and signalHandler.getAnAccess() = init.getAFieldExpr(handlerField).getAChild*()
+ // ignore if signals are blocked via sa_mask
+ and not exists(Field mask | mask.getName() = "sa_mask" and exists(init.getAFieldExpr(mask)))
+ )
+ )
+ // ignore if signals are blocked via sa_mask
+ and not exists(ValueFieldAccess dfa |
+ dfa.getQualifier+() = sigactVar.getAnAccess()
+ and dfa.getTarget().getName() = "sa_mask"
)
)
}
-
-predicate isSignalHandlerField(FieldAccess fa) {
- fa.getTarget().getName() in ["__sa_handler", "__sa_sigaction", "sa_handler", "sa_sigaction"]
-}
-
-
-from FunctionCall fc, Function signalHandler, FieldAccess fa
+
+from FunctionCall fc, Function signalHandler
where
- fc.getTarget().getName() = "signal" and
- signalHandler.getName() = fc.getArgument(1).toString() and
isAsyncUnsafe(signalHandler)
- or
- fc.getTarget().getName() = "sigaction" and
- isSignalHandlerField(fa) and
- signalHandler = fa.getTarget().getAnAssignedValue().(AddressOfExpr).getAddressable() and
- isAsyncUnsafe(signalHandler)
-select signalHandler,
- "is a non-trivial signal handler that may be using functions that are not async-safe."
\ No newline at end of file
+ and (
+ isSignal(fc, signalHandler)
+ or
+ isSigaction(fc, signalHandler)
+ )
+select signalHandler, "is a non-trivial signal handler that may be using not async-safe functions. Registered by $@", fc, fc.toString()
+
diff --git a/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.c b/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.c
index 32b6b43..238ff02 100644
--- a/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.c
+++ b/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.c
@@ -6,6 +6,7 @@ void transitive_call() {
printf("UNSAFE");
}
void transitive_call3() {
+ // TODO: decide which functions are exploitable
int *x = (int*)malloc(16);
free(x);
}
@@ -49,6 +50,7 @@ int main() {
}
// Another unsafe example
+ // TODO: decide which signals are exploitable and should produce findings
struct sigaction act = { 0 };
act.sa_flags = SA_SIGINFO;
act.sa_sigaction = &unsafe_handler2;
@@ -57,13 +59,40 @@ int main() {
_Exit(EXIT_FAILURE);
}
- struct sigaction act2 = { 0 };
- act2.sa_flags = SA_SIGINFO;
- act2.sa_handler = &unsafe_handler3;
+ // Yet another unsafe
+ struct sigaction act2 = {.sa_flags = SA_SIGINFO, .sa_sigaction = unsafe_handler2};
if (sigaction(SIGALRM, &act2, NULL) == -1) {
perror("sigaction 2");
_Exit(EXIT_FAILURE);
}
+ // Let's call these safe, because some signals are blocked
+ // TODO: not every masked signals should indicate safe signal handling
+ sigset_t sigset;
+ sigemptyset(&sigset);
+ sigaddset(&sigset, SIGALRM);
+ struct sigaction act3 = {.sa_flags = SA_SIGINFO, .sa_sigaction = unsafe_handler2, .sa_mask = sigset};
+ if (sigaction(SIGALRM, &act3, NULL) == -1) {
+ perror("sigaction 3");
+ _Exit(EXIT_FAILURE);
+ }
+
+ struct sigaction act4 = {.sa_flags = SA_SIGINFO, .sa_sigaction = unsafe_handler2};
+ act4.sa_mask = sigset;
+ if (sigaction(SIGSEGV, &act4, NULL) == -1) {
+ perror("sigaction 4");
+ _Exit(EXIT_FAILURE);
+ }
+
+ // TODO: this example should be not marked as finding
+ struct sigaction act5 = {.sa_mask = sigset};
+ act5.sa_flags = SA_SIGINFO;
+ act5.sa_sigaction = &unsafe_handler2;
+ if (sigaction(SIGSEGV, &act5, NULL) == -1) {
+ perror("sigaction 5");
+ _Exit(EXIT_FAILURE);
+ }
+
+
return 0;
}
\ No newline at end of file
diff --git a/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.expected b/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.expected
index e51cab2..dc9c3b0 100644
--- a/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.expected
+++ b/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.expected
@@ -1,3 +1,4 @@
-| AsyncUnsafeSignalHandler.c:23:6:23:19 | unsafe_handler | is a non-trivial signal handler that may be using functions that are not async-safe. |
-| AsyncUnsafeSignalHandler.c:27:6:27:20 | unsafe_handler2 | is a non-trivial signal handler that may be using functions that are not async-safe. |
-| AsyncUnsafeSignalHandler.c:34:6:34:20 | unsafe_handler3 | is a non-trivial signal handler that may be using functions that are not async-safe. |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
From 4e7e937bf49a375d1286b8e5d939a6d1f16528e7 Mon Sep 17 00:00:00 2001
From: GrosQuildu
Date: Fri, 5 Jul 2024 12:59:46 +0200
Subject: [PATCH 2/5] better qlhelp, ms_mask only changes msg
---
.../AsyncUnsafeSignalHandler.qhelp | 25 ++++++++-
.../AsyncUnsafeSignalHandler.ql | 35 ++++++++----
cpp/test/include/libc/signal.h | 13 +++--
cpp/test/include/libc/stdarg.h | 25 +++++++++
cpp/test/include/libc/stdlib.h | 1 +
cpp/test/include/libc/string_stubs.h | 5 ++
cpp/test/include/libc/unistd.h | 22 ++++++++
.../AsyncUnsafeSignalHandler.c | 53 +++++++++++++++++++
8 files changed, 160 insertions(+), 19 deletions(-)
create mode 100644 cpp/test/include/libc/stdarg.h
create mode 100644 cpp/test/include/libc/unistd.h
diff --git a/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.qhelp b/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.qhelp
index cc1130f..3394cf5 100644
--- a/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.qhelp
+++ b/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.qhelp
@@ -7,9 +7,23 @@
This is a CodeQL query constructed to find signal handlers that are performing async unsafe operations.
+
+ Because a signal may be delivered at any moment, e.g., in the middle of a malloc, the handler shouldn't touch
+ the program's internal state.
+
+
The kernel defines a list of async-safe signal functions in its man page.
- Any signal handler that performs operations that are not safe asynchronously may be vulnerable.
+ Any signal handler that performs operations that are not safe for asynchronous execution may be vulnerable.
+
+
+
+ Moreover, signal handlers may be re-entered. Handlers' logic should take that possibility into account.
+
+
+
+ If the issue is exploitable depends on attacker's ability to deliver the signal.
+ Remote attacks may be limitted to some signals (like SIGALRM and SIGURG), while local attacks could use all signals.
@@ -17,17 +31,24 @@
Attempt to keep signal handlers as simple as possible. Only call async-safe functions from signal handlers.
+
+Block delivery of new signals inside signal handlers to prevent handler re-entrancy issues.
+
- In this example, while both syntatically valid, a correct handler is defined in the correct_handler
function and sets a flag. The function calls log_message
, a async unsafe function, within the main loop.
+ In this example, while both syntatically valid, a correct handler is defined in the correct_handler
function and sets a flag.
+ The function calls log_message
, a async unsafe function, within the main loop.
+
+ Michal Zalewski, "Delivering Signals for Fun and Profit"
+
SEI CERT C Coding Standard "SIG30-C. Call only asynchronous-safe functions within signal handlers"
diff --git a/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.ql b/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.ql
index 9003a41..2600c0b 100644
--- a/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.ql
+++ b/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.ql
@@ -57,7 +57,7 @@ predicate isSignal(FunctionCall signalCall, Function signalHandler) {
* struct sigaction sigactVar = ...
* sigaction(SIGX, &sigactVar, ...)
*/
-predicate isSigaction(FunctionCall sigactionCall, Function signalHandler) {
+predicate isSigaction(FunctionCall sigactionCall, Function signalHandler, boolean isReentrancyBlocked) {
exists(Variable sigactVar, Struct sigactStruct, Field handlerField |
sigactionCall.getTarget().getName() = "sigaction"
and sigactionCall.getArgument(1).getAChild*() = sigactVar.getAnAccess()
@@ -84,25 +84,40 @@ predicate isSigaction(FunctionCall sigactionCall, Function signalHandler) {
varDec.getVariable() = sigactVar
and sigactVar.getInitializer().getExpr() = init
and signalHandler.getAnAccess() = init.getAFieldExpr(handlerField).getAChild*()
- // ignore if signals are blocked via sa_mask
- and not exists(Field mask | mask.getName() = "sa_mask" and exists(init.getAFieldExpr(mask)))
+
+ // new signals are blocked via sa_mask
+ and if exists(Field mask | mask.getName() = "sa_mask" and exists(init.getAFieldExpr(mask))) then
+ isReentrancyBlocked = true
+ else
+ isReentrancyBlocked = false
)
)
- // ignore if signals are blocked via sa_mask
- and not exists(ValueFieldAccess dfa |
+
+ // new signals are blocked via sa_mask
+ and if (isReentrancyBlocked = true or exists(ValueFieldAccess dfa |
dfa.getQualifier+() = sigactVar.getAnAccess()
and dfa.getTarget().getName() = "sa_mask"
- )
+ )) then
+ isReentrancyBlocked = true
+ else
+ isReentrancyBlocked = false
)
}
+
+string fmtMsg(boolean isReentrancyBlocked) {
+ (isReentrancyBlocked = true and result = "")
+ or
+ (isReentrancyBlocked = false and result = "Delivery of new signals may be not blocked when the handler executes. ")
+}
-from FunctionCall fc, Function signalHandler
+from FunctionCall fc, Function signalHandler, boolean isReentrancyBlocked
where
isAsyncUnsafe(signalHandler)
and (
- isSignal(fc, signalHandler)
+ (isSignal(fc, signalHandler) and isReentrancyBlocked = false)
or
- isSigaction(fc, signalHandler)
+ isSigaction(fc, signalHandler, isReentrancyBlocked)
)
-select signalHandler, "is a non-trivial signal handler that may be using not async-safe functions. Registered by $@", fc, fc.toString()
+select signalHandler, "is a non-trivial signal handler that uses not async-safe functions. " + fmtMsg(isReentrancyBlocked) +
+ "Handler is registered by $@", fc, fc.toString()
diff --git a/cpp/test/include/libc/signal.h b/cpp/test/include/libc/signal.h
index 5ade866..5b966bb 100644
--- a/cpp/test/include/libc/signal.h
+++ b/cpp/test/include/libc/signal.h
@@ -3,27 +3,29 @@
#ifndef HEADER_SIGNAL_STUB_H
#define HEADER_SIGNAL_STUB_H
-#ifdef __cplusplus
-extern "C" {
-#endif
+
#define SIGALRM 14
#define SIGSEGV 11
+#define SIGTERM 15
#define SIG_ERR -1
#define EXIT_FAILURE 2
-#define SA_SIGINFO 0x00000004
+#define SA_SIGINFO 4
+{} // to silent error from codeql's extractor
typedef void (*sighandler_t)(int);
extern int signal(int, sighandler_t);
typedef struct {
unsigned long sig[64];
} sigset_t;
+
typedef struct {
int si_signo; /* Signal number */
int si_errno; /* An errno value */
int si_code; /* Signal code */
} siginfo_t;
+
struct sigaction {
void (*sa_handler)(int);
void (*sa_sigaction)(int, siginfo_t *, void *);
@@ -35,9 +37,6 @@ struct sigaction {
extern int sigaction(int signum, const struct sigaction *act, struct sigaction *oldact);
extern int kill(int pid, int sig);
-#ifdef __cplusplus
-}
-#endif
#endif
diff --git a/cpp/test/include/libc/stdarg.h b/cpp/test/include/libc/stdarg.h
new file mode 100644
index 0000000..c9f353f
--- /dev/null
+++ b/cpp/test/include/libc/stdarg.h
@@ -0,0 +1,25 @@
+#ifndef USE_HEADERS
+
+#ifndef HEADER_STDARG_STUB_H
+#define HEADER_STDARG_STUB_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+typedef void *va_list;
+#define va_start(ap, parmN)
+#define va_end(ap)
+#define va_arg(ap, type) ((type)0)
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif
+
+#else // --- else USE_HEADERS
+
+#include
+
+#endif // --- end USE_HEADERS
\ No newline at end of file
diff --git a/cpp/test/include/libc/stdlib.h b/cpp/test/include/libc/stdlib.h
index cbacdf8..cc18de3 100644
--- a/cpp/test/include/libc/stdlib.h
+++ b/cpp/test/include/libc/stdlib.h
@@ -16,6 +16,7 @@ long lrand48(void) {
}
void _Exit(int);
+void exit(int);
void free(void*);
void *malloc(unsigned long);
diff --git a/cpp/test/include/libc/string_stubs.h b/cpp/test/include/libc/string_stubs.h
index e7e83ce..9310e50 100644
--- a/cpp/test/include/libc/string_stubs.h
+++ b/cpp/test/include/libc/string_stubs.h
@@ -24,6 +24,10 @@ extern int wprintf(const wchar_t * format, ...);
extern wchar_t* wcscpy(wchar_t * s1, const wchar_t * s2);
extern void perror(const char *s);
+extern void openlog(const char*, int, int);
+extern void syslog(int, const char*, ...);
+extern void closelog(void)
+
#ifdef __cplusplus
}
#endif
@@ -37,6 +41,7 @@ extern void perror(const char *s);
#include
#include
#include
+#include
#define strcpy_s strcpy
#define _mbsncat strncat
diff --git a/cpp/test/include/libc/unistd.h b/cpp/test/include/libc/unistd.h
new file mode 100644
index 0000000..7163a23
--- /dev/null
+++ b/cpp/test/include/libc/unistd.h
@@ -0,0 +1,22 @@
+#ifndef USE_HEADERS
+
+#ifndef HEADER_UNISTD_STUB_H
+#define HEADER_UNISTD_STUB_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+void _exit(int);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif
+
+#else // --- else USE_HEADERS
+
+#include
+
+#endif // --- end USE_HEADERS
\ No newline at end of file
diff --git a/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.c b/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.c
index 238ff02..b63e863 100644
--- a/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.c
+++ b/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.c
@@ -1,6 +1,8 @@
#include "../../../include/libc/string_stubs.h"
#include "../../../include/libc/signal.h"
#include "../../../include/libc/stdlib.h"
+#include "../../../include/libc/unistd.h"
+#include "../../../include/libc/stdarg.h"
void transitive_call() {
printf("UNSAFE");
@@ -36,6 +38,53 @@ void unsafe_handler3(int signal) {
transitive_call3();
}
+// data flow case, regresshion-like
+#define sigdie(...) do_logging_and_die(__FILE__, NULL, __VA_ARGS__)
+
+static void do_log(int level, const char *suffix, const char *fmt, va_list args) {
+ int pri = 0;
+ openlog(suffix, 1, 2);
+ // the unsafe call from the handler
+ syslog(pri, "%.500s", fmt);
+ closelog();
+}
+
+static const int level = 0;
+void logv(const char *file, const char *suffix, const char *fmt, va_list args) {
+ char tmp[] = "sufsuf: ";
+ suffix = tmp;
+ if(!args) {
+ suffix = &tmp[3];
+ }
+ do_log(level, suffix, fmt, args);
+}
+
+void do_logging_and_die(const char *file, const char *suffix, const char *fmt, ...) {
+ va_list args;
+ va_start(args, fmt);
+ logv(file, suffix, fmt, args);
+ va_end(args);
+ _exit(1);
+}
+
+static void df_handler(int sig) {
+ if (2*sig % 5 == 3) {
+ _exit(1);
+ }
+ sigdie("some log %d", sig);
+}
+
+sighandler_t register_signal(int signum, sighandler_t handler) {
+ struct sigaction sa, osa;
+ memset(&sa, 0, sizeof(sa));
+ sa.sa_handler = handler;
+ sigfillset(&sa.sa_mask);
+ if (sigaction(signum, &sa, &osa) == -1) {
+ return NULL;
+ }
+ return osa.sa_handler;
+}
+
int main() {
// Register the safe signal handler
if (signal(SIGALRM, safe_handler) == SIG_ERR) {
@@ -93,6 +142,10 @@ int main() {
_Exit(EXIT_FAILURE);
}
+ // indirect handler registration
+ if(register_signal(SIGALRM, df_handler)) {
+ _exit(EXIT_FAILURE);
+ }
return 0;
}
\ No newline at end of file
From 3dccc291a4dda89a790c30e7c9e5e58660fc99b1 Mon Sep 17 00:00:00 2001
From: GrosQuildu
Date: Fri, 5 Jul 2024 13:48:00 +0200
Subject: [PATCH 3/5] blocked signals fixed
---
.../AsyncUnsafeSignalHandler.ql | 57 ++++++++++---------
.../AsyncUnsafeSignalHandler.c | 13 +++--
2 files changed, 36 insertions(+), 34 deletions(-)
diff --git a/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.ql b/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.ql
index 2600c0b..ea7e626 100644
--- a/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.ql
+++ b/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.ql
@@ -57,8 +57,8 @@ predicate isSignal(FunctionCall signalCall, Function signalHandler) {
* struct sigaction sigactVar = ...
* sigaction(SIGX, &sigactVar, ...)
*/
-predicate isSigaction(FunctionCall sigactionCall, Function signalHandler, boolean isReentrancyBlocked) {
- exists(Variable sigactVar, Struct sigactStruct, Field handlerField |
+predicate isSigaction(FunctionCall sigactionCall, Function signalHandler, Variable sigactVar) {
+ exists(Struct sigactStruct, Field handlerField |
sigactionCall.getTarget().getName() = "sigaction"
and sigactionCall.getArgument(1).getAChild*() = sigactVar.getAnAccess()
@@ -79,45 +79,46 @@ predicate isSigaction(FunctionCall sigactionCall, Function signalHandler, boolea
and dfa.getQualifier+() = sigactVar.getAnAccess()
)
or
- exists(VariableDeclarationEntry varDec, ClassAggregateLiteral init |
+ exists(ClassAggregateLiteral initLit |
// struct sigaction sigactVar = {.sa_sigaction = signalHandler};
- varDec.getVariable() = sigactVar
- and sigactVar.getInitializer().getExpr() = init
- and signalHandler.getAnAccess() = init.getAFieldExpr(handlerField).getAChild*()
-
- // new signals are blocked via sa_mask
- and if exists(Field mask | mask.getName() = "sa_mask" and exists(init.getAFieldExpr(mask))) then
- isReentrancyBlocked = true
- else
- isReentrancyBlocked = false
+ // varDec.getVariable() = sigactVar
+ sigactVar.getInitializer().getExpr() = initLit
+ and signalHandler.getAnAccess() = initLit.getAFieldExpr(handlerField).getAChild*()
)
)
-
- // new signals are blocked via sa_mask
- and if (isReentrancyBlocked = true or exists(ValueFieldAccess dfa |
- dfa.getQualifier+() = sigactVar.getAnAccess()
- and dfa.getTarget().getName() = "sa_mask"
- )) then
- isReentrancyBlocked = true
- else
- isReentrancyBlocked = false
)
}
-string fmtMsg(boolean isReentrancyBlocked) {
- (isReentrancyBlocked = true and result = "")
+predicate isSignalDeliveryBlocked(Variable sigactVar) {
+ // TODO: should only find writes and for specific signals
+ exists(ValueFieldAccess dfa |
+ dfa.getQualifier+() = sigactVar.getAnAccess() and dfa.getTarget().getName() = "sa_mask"
+ )
or
- (isReentrancyBlocked = false and result = "Delivery of new signals may be not blocked when the handler executes. ")
+ exists(Field mask |
+ mask.getName() = "sa_mask"
+ and exists(sigactVar.getInitializer().getExpr().(ClassAggregateLiteral).getAFieldExpr(mask))
+ )
+}
+
+string deliveryNotBlockedMsg() {
+ result = "Delivery of new signals may be not blocked when the handler executes. "
}
-from FunctionCall fc, Function signalHandler, boolean isReentrancyBlocked
+from FunctionCall fc, Function signalHandler, string msg
where
isAsyncUnsafe(signalHandler)
and (
- (isSignal(fc, signalHandler) and isReentrancyBlocked = false)
+ (isSignal(fc, signalHandler) and msg = deliveryNotBlockedMsg())
or
- isSigaction(fc, signalHandler, isReentrancyBlocked)
+ exists(Variable sigactVar |
+ isSigaction(fc, signalHandler, sigactVar)
+ and if isSignalDeliveryBlocked(sigactVar) then
+ msg = ""
+ else
+ msg = deliveryNotBlockedMsg()
+ )
)
-select signalHandler, "is a non-trivial signal handler that uses not async-safe functions. " + fmtMsg(isReentrancyBlocked) +
+select signalHandler, "is a non-trivial signal handler that uses not async-safe functions. " + msg +
"Handler is registered by $@", fc, fc.toString()
diff --git a/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.c b/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.c
index b63e863..4473208 100644
--- a/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.c
+++ b/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.c
@@ -92,13 +92,13 @@ int main() {
_Exit(EXIT_FAILURE);
}
- // Unsafe example
+ // Unsafe example 1
if (signal(SIGALRM, unsafe_handler) == SIG_ERR) {
perror("Unable to catch SIGALRM");
_Exit(EXIT_FAILURE);
}
- // Another unsafe example
+ // Unsafe example 2
// TODO: decide which signals are exploitable and should produce findings
struct sigaction act = { 0 };
act.sa_flags = SA_SIGINFO;
@@ -108,14 +108,14 @@ int main() {
_Exit(EXIT_FAILURE);
}
- // Yet another unsafe
+ // Unsafe example 3
struct sigaction act2 = {.sa_flags = SA_SIGINFO, .sa_sigaction = unsafe_handler2};
if (sigaction(SIGALRM, &act2, NULL) == -1) {
perror("sigaction 2");
_Exit(EXIT_FAILURE);
}
- // Let's call these safe, because some signals are blocked
+ // Unsafe example 4
// TODO: not every masked signals should indicate safe signal handling
sigset_t sigset;
sigemptyset(&sigset);
@@ -126,6 +126,7 @@ int main() {
_Exit(EXIT_FAILURE);
}
+ // Unsafe example 5
struct sigaction act4 = {.sa_flags = SA_SIGINFO, .sa_sigaction = unsafe_handler2};
act4.sa_mask = sigset;
if (sigaction(SIGSEGV, &act4, NULL) == -1) {
@@ -133,7 +134,7 @@ int main() {
_Exit(EXIT_FAILURE);
}
- // TODO: this example should be not marked as finding
+ // Unsafe example 6
struct sigaction act5 = {.sa_mask = sigset};
act5.sa_flags = SA_SIGINFO;
act5.sa_sigaction = &unsafe_handler2;
@@ -142,7 +143,7 @@ int main() {
_Exit(EXIT_FAILURE);
}
- // indirect handler registration
+ // Unsafe example 7, indirect handler registration
if(register_signal(SIGALRM, df_handler)) {
_exit(EXIT_FAILURE);
}
From 0cf71fd622e04e19a7c257434b9a6b08815b0e3b Mon Sep 17 00:00:00 2001
From: GrosQuildu
Date: Fri, 5 Jul 2024 15:50:12 +0200
Subject: [PATCH 4/5] add data flow
---
.../AsyncUnsafeSignalHandler.ql | 124 +++++++++++-------
cpp/test/include/libc/signal.h | 6 +-
.../AsyncUnsafeSignalHandler.c | 32 ++++-
.../AsyncUnsafeSignalHandler.expected | 12 +-
4 files changed, 114 insertions(+), 60 deletions(-)
diff --git a/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.ql b/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.ql
index ea7e626..ac5a0d0 100644
--- a/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.ql
+++ b/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.ql
@@ -10,47 +10,68 @@
* @group security
*/
- import cpp
+import cpp
+import semmle.code.cpp.dataflow.new.DataFlow
- /* List from https://man7.org/linux/man-pages/man3/stdio.3.html */
- class StdioFunction extends Function {
- StdioFunction() {
- this.getName() in [
- "fopen", "freopen", "fflush", "fclose", "fread", "fwrite", "fgetc", "fgets", "fputc",
- "fputs", "getc", "getchar", "gets", "putc", "putchar", "puts", "ungetc", "fprintf",
- "fscanf", "printf", "scanf", "sprintf", "sscanf", "vprintf", "vfprintf", "vsprintf",
- "fgetpos", "fsetpos", "ftell", "fseek", "rewind", "clearerr", "feof", "ferror", "perror",
- "setbuf", "setvbuf", "remove", "rename", "tmpfile", "tmpnam",
- ]
- }
- }
-
- /* List from https://man7.org/linux/man-pages/man3/syslog.3.html */
- class SyslogFunction extends Function {
- SyslogFunction() { this.getName() in ["syslog", "vsyslog",] }
- }
-
- /* List from https://man7.org/linux/man-pages/man0/stdlib.h.0p.html */
- class StdlibFunction extends Function {
- StdlibFunction() { this.getName() in ["malloc", "calloc", "realloc", "free",] }
- }
-
- predicate isAsyncUnsafe(Function signalHandler) {
- exists(Function f |
- signalHandler.calls+(f) and
- (
- f instanceof StdioFunction or
- f instanceof SyslogFunction or
- f instanceof StdlibFunction
- )
- )
- }
+/* List from https://man7.org/linux/man-pages/man3/stdio.3.html */
+class StdioFunction extends Function {
+ StdioFunction() {
+ this.getName() in [
+ "fopen", "freopen", "fflush", "fclose", "fread", "fwrite", "fgetc", "fgets", "fputc",
+ "fputs", "getc", "getchar", "gets", "putc", "putchar", "puts", "ungetc", "fprintf",
+ "fscanf", "printf", "scanf", "sprintf", "sscanf", "vprintf", "vfprintf", "vsprintf",
+ "fgetpos", "fsetpos", "ftell", "fseek", "rewind", "clearerr", "feof", "ferror", "perror",
+ "setbuf", "setvbuf", "remove", "rename", "tmpfile", "tmpnam",
+ ]
+ }
+}
+
+/* List from https://man7.org/linux/man-pages/man3/syslog.3.html */
+class SyslogFunction extends Function {
+ SyslogFunction() { this.getName() in ["syslog", "vsyslog",] }
+}
+
+/* List from https://man7.org/linux/man-pages/man0/stdlib.h.0p.html */
+class StdlibFunction extends Function {
+ StdlibFunction() { this.getName() in ["malloc", "calloc", "realloc", "free",] }
+}
+
+predicate isAsyncUnsafe(Function signalHandler) {
+ exists(Function f |
+ signalHandler.calls+(f) and
+ (
+ f instanceof StdioFunction or
+ f instanceof SyslogFunction or
+ f instanceof StdlibFunction
+ )
+ )
+}
+
+/**
+ * Flows from any function ptr
+ */
+module HandlerToSignalConfiguration implements DataFlow::ConfigSig {
+ predicate isSource(DataFlow::Node source) {
+ source.asExpr() = any(Function f || f.getAnAccess())
+ }
+
+ predicate isSink(DataFlow::Node sink) {
+ sink = sink
+ }
+}
+module HandlerToSignal = DataFlow::Global;
-// signal(SIGX, signalHandler)
+/**
+ * signal(SIGX, signalHandler)
+ */
predicate isSignal(FunctionCall signalCall, Function signalHandler) {
signalCall.getTarget().getName() = "signal"
- and signalHandler.getAnAccess() = signalCall.getArgument(1).getAChild*()
+ and exists(DataFlow::Node source, DataFlow::Node sink |
+ HandlerToSignal::flow(source, sink)
+ and source.asExpr() = signalHandler.getAnAccess()
+ and sink.asExpr() = signalCall.getArgument(1)
+ )
}
/**
@@ -58,7 +79,7 @@ predicate isSignal(FunctionCall signalCall, Function signalHandler) {
* sigaction(SIGX, &sigactVar, ...)
*/
predicate isSigaction(FunctionCall sigactionCall, Function signalHandler, Variable sigactVar) {
- exists(Struct sigactStruct, Field handlerField |
+ exists(Struct sigactStruct, Field handlerField, DataFlow::Node source, DataFlow::Node sink |
sigactionCall.getTarget().getName() = "sigaction"
and sigactionCall.getArgument(1).getAChild*() = sigactVar.getAnAccess()
@@ -70,27 +91,36 @@ predicate isSigaction(FunctionCall sigactionCall, Function signalHandler, Variab
or
exists(Union u | u.getAField() = handlerField and u = sigactStruct.getAField().getType())
)
+
and (
- exists(Assignment a, ValueFieldAccess dfa |
- // sigactVar.sa_handler = &signalHandler
- a.getLValue() = dfa.getAChild*()
- and a.getRValue().getAChild*() = signalHandler.getAnAccess()
- and dfa.getTarget() = handlerField
- and dfa.getQualifier+() = sigactVar.getAnAccess()
+ // sigactVar.sa_handler = &signalHandler
+ exists(Assignment a, ValueFieldAccess vfa |
+ vfa.getTarget() = handlerField
+ and vfa.getQualifier+() = sigactVar.getAnAccess()
+ and a.getLValue() = vfa.getAChild*()
+
+ and source.asExpr() = signalHandler.getAnAccess()
+ and sink.asExpr() = a.getRValue()
+ and HandlerToSignal::flow(source, sink)
)
or
+ // struct sigaction sigactVar = {.sa_sigaction = signalHandler};
exists(ClassAggregateLiteral initLit |
- // struct sigaction sigactVar = {.sa_sigaction = signalHandler};
- // varDec.getVariable() = sigactVar
sigactVar.getInitializer().getExpr() = initLit
- and signalHandler.getAnAccess() = initLit.getAFieldExpr(handlerField).getAChild*()
+ and source.asExpr() = signalHandler.getAnAccess()
+ and sink.asExpr() = initLit.getAFieldExpr(handlerField).getAChild*()
+ and HandlerToSignal::flow(source, sink)
)
)
)
}
+/**
+ * Determine if new signals are blocked
+ * TODO: should only find writes and only for correct (or all) signals
+ * TODO: should detect other block mechanisms
+ */
predicate isSignalDeliveryBlocked(Variable sigactVar) {
- // TODO: should only find writes and for specific signals
exists(ValueFieldAccess dfa |
dfa.getQualifier+() = sigactVar.getAnAccess() and dfa.getTarget().getName() = "sa_mask"
)
diff --git a/cpp/test/include/libc/signal.h b/cpp/test/include/libc/signal.h
index 5b966bb..fcc0bf8 100644
--- a/cpp/test/include/libc/signal.h
+++ b/cpp/test/include/libc/signal.h
@@ -3,8 +3,6 @@
#ifndef HEADER_SIGNAL_STUB_H
#define HEADER_SIGNAL_STUB_H
-
-
#define SIGALRM 14
#define SIGSEGV 11
#define SIGTERM 15
@@ -13,8 +11,8 @@
#define SA_SIGINFO 4
{} // to silent error from codeql's extractor
-typedef void (*sighandler_t)(int);
-extern int signal(int, sighandler_t);
+typedef void (*sig_t)(int);
+extern int signal(int, sig_t);
typedef struct {
unsigned long sig[64];
diff --git a/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.c b/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.c
index 4473208..72c0b9c 100644
--- a/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.c
+++ b/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.c
@@ -23,6 +23,12 @@ void safe_handler(int sig) {
}
}
+void safe_handler2(int signo, siginfo_t *info, void *context) {
+ if (signo == SIGALRM) {
+ kill(1, SIGALRM);
+ }
+}
+
void unsafe_handler(int sig) {
transitive_call();
}
@@ -34,7 +40,7 @@ void unsafe_handler2(int signo, siginfo_t *info, void *context) {
transitive_call2();
}
-void unsafe_handler3(int signal) {
+void unsafe_handler3(int signo, siginfo_t *info, void *context) {
transitive_call3();
}
@@ -74,7 +80,7 @@ static void df_handler(int sig) {
sigdie("some log %d", sig);
}
-sighandler_t register_signal(int signum, sighandler_t handler) {
+sig_t register_signal(int signum, sig_t handler) {
struct sigaction sa, osa;
memset(&sa, 0, sizeof(sa));
sa.sa_handler = handler;
@@ -92,6 +98,15 @@ int main() {
_Exit(EXIT_FAILURE);
}
+ // Safe handler 2
+ struct sigaction actG2 = {0};
+ actG2.sa_flags = SA_SIGINFO;
+ actG2.sa_sigaction = &safe_handler2;
+ if (sigaction(SIGSEGV, &actG2, NULL) == -1) {
+ perror("sigaction");
+ _Exit(EXIT_FAILURE);
+ }
+
// Unsafe example 1
if (signal(SIGALRM, unsafe_handler) == SIG_ERR) {
perror("Unable to catch SIGALRM");
@@ -127,7 +142,7 @@ int main() {
}
// Unsafe example 5
- struct sigaction act4 = {.sa_flags = SA_SIGINFO, .sa_sigaction = unsafe_handler2};
+ struct sigaction act4 = {.sa_flags = SA_SIGINFO, .sa_sigaction = unsafe_handler3};
act4.sa_mask = sigset;
if (sigaction(SIGSEGV, &act4, NULL) == -1) {
perror("sigaction 4");
@@ -137,13 +152,20 @@ int main() {
// Unsafe example 6
struct sigaction act5 = {.sa_mask = sigset};
act5.sa_flags = SA_SIGINFO;
- act5.sa_sigaction = &unsafe_handler2;
+ act5.sa_sigaction = &unsafe_handler3;
if (sigaction(SIGSEGV, &act5, NULL) == -1) {
perror("sigaction 5");
_Exit(EXIT_FAILURE);
}
- // Unsafe example 7, indirect handler registration
+ // Unsafe example 7
+ sig_t wrapper = unsafe_handler;
+ if (signal(SIGALRM, wrapper) == SIG_ERR) {
+ perror("Unable to catch SIGALRM");
+ _Exit(EXIT_FAILURE);
+ }
+
+ // Unsafe example 8, indirect handler registration
if(register_signal(SIGALRM, df_handler)) {
_exit(EXIT_FAILURE);
}
diff --git a/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.expected b/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.expected
index dc9c3b0..71a5e34 100644
--- a/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.expected
+++ b/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.expected
@@ -1,4 +1,8 @@
-| 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 |
-| 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 |
-| 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 |
-| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
From 04e5d42e3ff8624ecb3ece6119f42f9bdf6da601 Mon Sep 17 00:00:00 2001
From: GrosQuildu
Date: Fri, 5 Jul 2024 16:02:05 +0200
Subject: [PATCH 5/5] polishing msg
---
.../AsyncUnsafeSignalHandler.ql | 6 +++---
.../AsyncUnsafeSignalHandler.expected | 16 ++++++++--------
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.ql b/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.ql
index ac5a0d0..2e1ff79 100644
--- a/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.ql
+++ b/cpp/src/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.ql
@@ -132,7 +132,7 @@ predicate isSignalDeliveryBlocked(Variable sigactVar) {
}
string deliveryNotBlockedMsg() {
- result = "Delivery of new signals may be not blocked when the handler executes. "
+ result = "Moreover, delivery of new signals may be not blocked. "
}
from FunctionCall fc, Function signalHandler, string msg
@@ -149,6 +149,6 @@ where
msg = deliveryNotBlockedMsg()
)
)
-select signalHandler, "is a non-trivial signal handler that uses not async-safe functions. " + msg +
- "Handler is registered by $@", fc, fc.toString()
+select signalHandler, "$@ is a non-trivial signal handler that uses not async-safe functions. " + msg +
+ "Handler is registered by $@.", signalHandler, signalHandler.toString(), fc, fc.toString()
diff --git a/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.expected b/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.expected
index 71a5e34..da13543 100644
--- a/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.expected
+++ b/cpp/test/query-tests/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.expected
@@ -1,8 +1,8 @@
-| 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 |
-| 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 |
-| 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 |
-| 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 |
-| 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 |
-| 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 |
-| 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 |
-| 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 |
+| AsyncUnsafeSignalHandler.c:32:6:32:19 | unsafe_handler | $@ is a non-trivial signal handler that uses not async-safe functions. Moreover, delivery of new signals may be not blocked. Handler is registered by $@. | AsyncUnsafeSignalHandler.c:32:6:32:19 | unsafe_handler | unsafe_handler | AsyncUnsafeSignalHandler.c:111:9:111:14 | call to signal | call to signal |
+| AsyncUnsafeSignalHandler.c:32:6:32:19 | unsafe_handler | $@ is a non-trivial signal handler that uses not async-safe functions. Moreover, delivery of new signals may be not blocked. Handler is registered by $@. | AsyncUnsafeSignalHandler.c:32:6:32:19 | unsafe_handler | unsafe_handler | AsyncUnsafeSignalHandler.c:163:9:163:14 | call to signal | call to signal |
+| 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:36:6:36:20 | unsafe_handler2 | unsafe_handler2 | AsyncUnsafeSignalHandler.c:139:9:139:17 | call to sigaction | call to sigaction |
+| AsyncUnsafeSignalHandler.c:36:6:36:20 | unsafe_handler2 | $@ is a non-trivial signal handler that uses not async-safe functions. Moreover, delivery of new signals may be not blocked. Handler is registered by $@. | AsyncUnsafeSignalHandler.c:36:6:36:20 | unsafe_handler2 | unsafe_handler2 | AsyncUnsafeSignalHandler.c:121:9:121:17 | call to sigaction | call to sigaction |
+| AsyncUnsafeSignalHandler.c:36:6:36:20 | unsafe_handler2 | $@ is a non-trivial signal handler that uses not async-safe functions. Moreover, delivery of new signals may be not blocked. Handler is registered by $@. | AsyncUnsafeSignalHandler.c:36:6:36:20 | unsafe_handler2 | unsafe_handler2 | AsyncUnsafeSignalHandler.c:128:9:128:17 | call to sigaction | call to sigaction |
+| 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:43:6:43:20 | unsafe_handler3 | unsafe_handler3 | AsyncUnsafeSignalHandler.c:147:9:147:17 | call to sigaction | call to sigaction |
+| 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:43:6:43:20 | unsafe_handler3 | unsafe_handler3 | AsyncUnsafeSignalHandler.c:156:9:156:17 | call to sigaction | call to sigaction |
+| 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:76:13:76:22 | df_handler | df_handler | AsyncUnsafeSignalHandler.c:88:6:88:14 | call to sigaction | call to sigaction |