...
Signal handlers should be as concise as possible—ideally, unconditionally setting a flag and returning. This compliant solution sets a flag of type volatile sig_atomic_t and returns; the log_message() and free() functions are called directly from main().:
| Code Block | ||||
|---|---|---|---|---|
| ||||
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
enum { MAXLINE = 1024 };
volatile sig_atomic_t eflag = 0;
char *info = NULL;
void log_message(void) {
fprintf(stderr, info);
}
void handler(int signum) {
eflag = 1;
}
int main(void) {
if (signal(SIGINT, handler) == SIG_ERR) {
/* Handle error */
}
info = (char*)malloc(MAXLINE);
if (info == NULL) {
/* Handle error */
}
while (!eflag) {
/* Main loop program code */
log_message();
/* More program code */
}
log_message();
free(info);
info = NULL;
return 0;
}
|
...
However, an attacker can exploit this noncompliant code example by generating a SIGINT just before the second if statement in log_message(). This results in The result is that longjmp() transferring transfers control back to main(), where log_message() is called again. However, the first if statement would not be executed this time (because buf is not set to NULL as a result of the interrupt), and the program would write to the invalid memory location referenced by buf0.
...
In this compliant solution, the call to longjmp() is removed; the signal handler sets an error flag of type volatile sig_atomic_t instead.:
| Code Block | ||||
|---|---|---|---|---|
| ||||
#include <signal.h>
#include <stdlib.h>
enum { MAXLINE = 1024 };
volatile sig_atomic_t eflag = 0;
void handler(int signum) {
eflag = 1;
}
void log_message(char *info1, char *info2) {
static char *buf = NULL;
static size_t bufsize;
char buf0[MAXLINE];
if (buf == NULL) {
buf = buf0;
bufsize = sizeof(buf0);
}
/*
* Try to fit a message into buf, else re-allocate
* it on the heap and then log the message.
*/
if (buf == buf0) {
buf = NULL;
}
}
int main(void) {
if (signal(SIGINT, handler) == SIG_ERR) {
/* Handle error */
}
char *info1;
char *info2;
/* info1 and info2 are set by user input here */
while (!eflag) {
/* Main loop program code */
log_message(info1, info2);
/* More program code */
}
log_message(info1, info2);
return 0;
} |
...
In this compliant solution, the call to the raise() function inside handler() has been replaced by a direct call to log_msg().:
| Code Block | ||||
|---|---|---|---|---|
| ||||
#include <signal.h>
void log_msg(int signum) {
/* Log error message in some asynchronous-safe manner */
}
void handler(int signum) {
/* Do some handling specific to SIGINT */
log_msg(SIGUSR1);
}
int main(void) {
if (signal(SIGUSR1, log_msg) == SIG_ERR) {
/* Handle error */
}
if (signal(SIGINT, handler) == SIG_ERR) {
/* Handle error */
}
/* program code */
if (raise(SIGINT) != 0) {
/* Handle error */
}
/* More code */
return 0;
}
|
...
The POSIX standard is contradictory regarding raise() in signal handlers. The POSIX standard [Open Group 2004] prohibits signal handlers installed using signal() from calling the raise() function if the signal occurs as the result of calling the raise(), kill(), pthread_kill(), or sigqueue() functions. However, it allows the raise() function to be safely called within any signal handler. Consequently, it is not clear whether it is safe for POSIX applications to call raise() in signal handlers installed using signal(), but it is safe to call raise() in signal handlers installed using sigaction().
In this noncompliant code example, the signal handlers are installed using signal(), and raise() is called inside the signal handler.:
| Code Block | ||||
|---|---|---|---|---|
| ||||
#include <signal.h>
void log_msg(int signum) {
/* Log error message */
}
void handler(int signum) {
/* Do some handling specific to SIGINT */
if (raise(SIGUSR1) != 0) { /* violation */
/* Handle error */
}
}
int main(void) {
signal(SIGUSR1, log_msg);
signal(SIGINT, handler);
/* program code */
if (raise(SIGINT) != 0) {
/* Handle error */
}
/* More code */
return 0;
}
|
...
In this compliant solution, the signal handlers are installed using sigaction(), and so it is safe to use raise() within the signal handler.:
| Code Block | ||||
|---|---|---|---|---|
| ||||
#include <signal.h>
void log_msg(int signum) {
/* Log error message in some asynchronous-safe manner */
}
void handler(int signum) {
/* Do some handling specific to SIGINT */
if (raise(SIGUSR1) != 0) {
/* Handle error */
}
}
int main(void) {
struct sigaction act;
act.sa_flags = 0;
if (sigemptyset(&act.sa_mask) != 0) {
/* Handle error */
}
act.sa_handler = log_msg;
if (sigaction(SIGUSR1, &act, NULL) != 0) {
/* Handle error */
}
act.sa_handler = handler;
if (sigaction(SIGINT, &act, NULL) != 0) {
/* Handle error */
}
/* program code */
if (raise(SIGINT) != 0) {
/* Handle error */
}
/* More code */
return 0;
}
|
...
Tool | Version | Checker | Description | ||||||
|---|---|---|---|---|---|---|---|---|---|
| Compass/ROSE | Can detect violations of the rule for single-file programs. | ||||||||
| 88 D | Fully implemented. | |||||||
Splint |
|
|
|
...