Avoid using signals to implement normal functionality. Signal handlers are severely limited in the actions they can perform in a portably secure manner. Their use should be reserved for abnormal events that can be serviced by little more than logging.
Noncompliant Code Example
This noncompliant code example uses signals as a means to pass state changes around in a multithreaded environment:
/* THREAD 1 */ int do_work(void) { /* ... */ kill(THR2_PID, SIGUSR1); } /* THREAD 2 */ volatile sig_atomic_t flag; void sigusr1_handler(int signum) { flag = 1; } int wait_and_work(void) { flag = 0; while (!flag) {} /* ... */ }
However, using signals for such functionality often leads to nonportable or otherwise complicated solutions.
This code illustrates one thread using a signal to wake up a second thread. Using an architecture's native thread library usually allows for a more sophisticated means of sending messages between threads.
Compliant Solution (POSIX)
A better solution, in this case, is to use condition variables. This code example uses a condition variable from the POSIX pthread
library [IEEE Std 1003.1:2013]:
#include <pthread.h> pthread_cond_t cond = PTHREAD_COND_INITIALIZER; pthread_mutex_t mut = PTHREAD_MUTEX_INITIALIZER; /* THREAD 1 */ int do_work(void) { int result; /* ... */ if ((result = pthread_mutex_lock(&mut)) != 0) { /* Handle error condition */ } if ((result = pthread_cond_signal(&cond,&mut)) != 0) { /* Handle error condition */ } if ((result = pthread_mutex_unlock(&mut)) != 0) { /* Handle error condition */ } } /* THREAD 2 */ int wait_and_work(void) { if ((result = pthread_mutex_lock(&mut)) != 0) { /* Handle error condition */ } while (/* Condition does not hold */) { if ((result = pthread_cond_wait(&cond, &mut)) != 0) { /* Handle error condition */ } /* ... */ } if ((result = pthread_mutex_unlock(&mut)) != 0) { /* Handle error condition */ } /* ... */ }
Compliant Solution (Windows)
This compliant solution uses a condition variable from the Win32 API [MSDN]:
#include <windows.h> /* * Note that the CRITICAL_SECTION must be initialized with * InitializeCriticalSection, and the CONDITION_VARIABLE must * be initialized with InitializeConditionVariable prior to * using them. */ CRITICAL_SECTION CritSection; CONDITION_VARIABLE ConditionVar; /* THREAD 1 */ int do_work(void) { /* ... */ WakeConditionVariable(&ConditionVar); } /* THREAD 2 */ int wait_and_work(void) { EnterCriticalSection(&CritSection); SleepConditionVariableCS(&ConditionVar, &CritSection, INFINITE); LeaveCriticalSection(&CritSection); /* ... */ }
Noncompliant Code Example
This noncompliant code example is from a signal race vulnerability in WU-FTPD v2.4 [Greenman 1997]:
void dologout(status) { if (logged_in) { (void) seteuid((uid_t)0); logwtmp(ttyline, "", ""); /* ... */ } _exit(status); } static void lostconn(int signo) { if (debug) syslog(LOG_DEBUG, "lost connection"); dologout(-1); } static void myoob(signo) { if (!transflag) return; /* ... */ if (strcmp(cp, "ABOR\r\n") == 0) { tmpline[0] = '\0'; reply(426, "Transfer aborted. Data connection closed."); reply(226, "Abort successful"); longjmp(urgcatch, 1); } /* ... */ } /* ... */ signal(SIGPIPE, lostconn); signal(SIGURG, myoob);
A serious exploit can occur if SIGURG
is caught immediately following the elevation of privileges in dologout()
. If the longjmp()
in the SIGURG
handler myoob()
is invoked, execution returns to the main processing loop with an effective UID of 0.
Please note that this code sample violates SIG30-C. Call only asynchronous-safe functions within signal handlers and SIG31-C. Do not access shared objects in signal handlers.
An immediate fix is to ensure that dologout()
cannot be interrupted by a SIGURG
:
void dologout(status) { /* * Prevent reception of SIGURG from resulting in a resumption * back to the main program loop. */ transflag = 0; if (logged_in) { (void) seteuid((uid_t)0); logwtmp(ttyline, "", ""); /* ... */ } _exit(status); }
A better solution is for myoob()
to set a failure flag of type volatile sig_atomic_t
that is periodically checked within the main loop:
volatile sig_atomic_t xfer_aborted = 0; static void myoob(signo) { /* ... */ if (strcmp(cp, "ABOR\r\n") == 0) { xfer_aborted = 1; } /* ... */ }
This solution, however, still violates SIG30-C. Call only asynchronous-safe functions within signal handlers and SIG31-C. Do not access shared objects in signal handlers.
Compliant Solution
A compliant solution (not shown) is to not use signals to signify lost connections and to design the system to have a robust error-handling mechanism (see ERR00-C. Adopt and implement a consistent and comprehensive error-handling policy).
Risk Assessment
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
SIG02-C | High | Probable | Medium | P12 | L1 |
Automated Detection
Tool | Version | Checker | Description |
---|---|---|---|
CodeSonar | 8.1p0 | BADFUNC.SIGNAL | Use of signal |
LDRA tool suite | 9.7.1 | 44 S | Enhanced Enforcement |
Parasoft C/C++test | 2023.1 | CERT_C-SIG02-a | The signal handling facilities of <signal.h> shall not be used |
PC-lint Plus | 1.4 | 586 | Assistance provided: reports use of the signal function |
Related Vulnerabilities
Using signals to implement normal functionality frequently results in the violation of one or more secure coding rules for signal handling.
Search for vulnerabilities resulting from the violation of this rule on the CERT website.
Related Guidelines
Bibliography
[Dowd 2006] | Chapter 13, "Synchronization and State" |
[Greenman 1997] | |
[IEEE Std 1003.1:2013] | XBD, Headers, <pthread> |
[MSDN] | Using Condition Variables |
7 Comments
Robert Seacord (Manager)
The examples in this recommendation talk about Inter Process Communication, but the actual example is inter thread communication. This is a bit of a mismatch that needs to be resolved.
When I read the start of this rule, I always expect to see a sendmail example where user functionality is implement through signals. I think this would make a better primary example, with perhaps the threads example a secondary example.
Alex Volkovitsky
any idea where we can find some sendmail code to reference?
Hallvard Furuseth
www.sendmail.org springs to mind... Look for sm_signal() calls, used to set signal handlers.
OTOH their "milter", whatever that is, uses a signal handling thread, which is a nice way to handle the issue: Ensure signals are delivered to that thread. It waits for them with sigwait() and can thus react to them immediately even though it does so outside a signal handler.
Alex Volkovitsky
ok... have been groking through their code and I can't seem to find anything particularly bad about their implementation...
sm_signal()
is basically justsigaction()
configured for the current system, and most of the signals are either set to either SIG_IGN, SIG_DFL, or a no-op... of the signals that are actually used they seem to just do basic signal handling thingsare you guys are talking about their
pend_signal()
/sm_releasesignal()
/sm_blocksignal()
functions?Edit: (after more groking)
sm_releasesignal()
/sm_blocksignal()
are just system specific versions ofsigblock()
/sigsetmask()
pend_signal()
is a way to keep signals from interrupting critical sections of, if a signal event happens inside of a critical section, the signal will pend itself until the nextSIGALRM
, once aSIGALRM
finally ticks outside of a critical section, it will run over all of the pending signals and execute them, since we are no longer in a critical section (by virtue ofsm_tick()
running), none of these should quit out and re-pend themselvesall in all pretty clever and nothing that I would deem inappropriate behavior for signals :/
David Svoboda
I would guess that sendmail's signal vuls (if any) were historical, and no longer in the current code.
Charles-Francois Natali
The first compliant solution (POSIX) has two problems:
1) it's not pthread_mutex_wait, but pthread_cond_wait
2) there should be a boolean predicate associated to the condition variable that's checked after return from pthread_cond_wait, because of the posibility of spurious wakeups:
Quoting man page:
David Svoboda
I've amended the code. Our Java wiki has this rule:
THI03-J. Always invoke wait() and await() methods inside a loop
We don't yet have a C/pthreads analogue to this rule, and need one to fully address the point you raise.