Day 49: Common C Code Vulnerabilities and Mitigations

Being able to spot vulnerabilities by looking at code is a key skill worth acquiring for a number of reasons, best one being that one can save hours of not going down fruitless rabbit holes by understanding the code better.

It can also be an employable quality exploit development companies look for in candidates, so if that’s your kind of thing, maybe today’s post can start to help you understand key vulnerabilities better and land you your next job.

Excerpt from a job posting

A lot of C vulnerabilities relate to buffer overflows. Buffers areas of memory set aside to hold data, when vulnerable code is written allows an exploit to write over other important values in memory, like the instructions that the CPU must execute next. C and C++ are susceptible to buffer overflows because they define strings as null-terminated arrays of characters, do not implicitly check bounds and provide standard library calls for strings that do not enforce bounds checking.

gets()

The fgets() function reads at most one less than the number of characters specified by size from the given stream and stores them in the string str. Reading stops when a newline character is found, at end-of-file or error. The newline, if any, is retained. If any characters are read and there is no error, a `\0' character is appended to end the string.

The gets() function is equivalent to fgets() with an infinite size and a stream of stdin, except that the newline character (if any) is not stored in the string. It is the caller’s responsibility to ensure that the input line, if any, is sufficiently short to fit in the string.

The gets() function cannot be used securely. Because of its lack of bounds checking, and the inability for the calling program to reliably determine the length of the next incoming line, the use of this function enables malicious users to arbitrarily change a running program’s functionality through a buffer overflow attack.

If you compile this you will see…

warning: the `gets' function is dangerous and should not be used.

Mitigation

strcpy() & stpcpy()

The stpcpy() and strcpy() functions copy the string src to dst (including the terminating `\0' character.) The stpncpy() and strncpy() functions copy at most len characters from src into dst. If src is less than len characters long, the remainder of dst is filled with `\0' characters. Otherwise, dst is not terminated. The source and destination strings should not overlap, as the behavior is undefined.

The strcpy() function is easily misused in a manner which enables malicious users to arbitrarily change a running program’s functionality through a buffer overflow attack.

Mitigation

The best way to mitigate this issue is to use strlcpy if it is readily available, it is also very simple to define it yourself if you need to:

You can also use strncpy which prevents buffer overflows but does not guarantee ‘\0’-termination.

strcat() and strcmp()

The functions strcat() and strcmp() are similarly vulnerable to strcpy(). To mitigate strcat() issues use strncat(). To mitigate strcmp() issues use strncpm().

strcat()

The strcat() function is easily misused in a manner which enables malicious users to arbitrarily change a running program’s functionality through a buffer overflow attack. Avoid using strcat(). Instead, use strncat() or strlcat() and ensure that no more characters are copied to the destination buffer than it can hold. Note that strncat() can also be problematic. It may be a security concern for a string to be truncated at all. Since the truncated string will not be as long as the original, it may refer to a completely different resource and usage of the truncated resource could result in very incorrect behaviour.

Example:

void
     foo(const char *arbitrary_string)
     {
             char onstack[8] = "";
#if defined(BAD)
             /*
              * This first strcat is bad behavior.  Do not use strcat!
              */
             (void)strcat(onstack, arbitrary_string);        /* BAD! */
     #elif defined(BETTER)
             /*
              * The following two lines demonstrate better use of
              * strncat().
              */
             (void)strncat(onstack, arbitrary_string,
                 sizeof(onstack) - strlen(onstack) - 1);
     #elif defined(BEST)
             /*
              * These lines are even more robust due to testing for
              * truncation.
              */
             if (strlen(arbitrary_string) + 1 >
                 sizeof(onstack) - strlen(onstack))
                     err(1, "onstack would be truncated");
             (void)strncat(onstack, arbitrary_string,
                 sizeof(onstack) - strlen(onstack) - 1);
     #endif
     }

sprintf()

The sprintf function does not check the buffer boundaries.

The sprintf() and vsprintf() functions are easily misused in a manner which enables malicious users to arbitrarily change a running program’s functionality through a buffer overflow attack. Because sprintf() and vsprintf() assume an infinitely long string, callers must be careful not to overflow the actual space; this is often hard to assure. For safety, programmers should use the snprintf() interface instead. For example:

void
 foo(const char *arbitrary_string, const char *and_another)
 {
 char onstack[8];
#ifdef BAD
 /*
 * This first sprintf is bad behavior. Do not use sprintf!
 */
 sprintf(onstack, “%s, %s”, arbitrary_string, and_another);
 #else
 /*
 * The following two lines demonstrate better use of
 * snprintf().
 */
 snprintf(onstack, sizeof(onstack), “%s, %s”, arbitrary_string,
 and_another);
 #endif
 }

The printf() and sprintf() family of functions are also easily misused in a manner allowing malicious users to arbitrarily change a running program’s functionality by either causing the program to print potentially sensitive data ``left on the stack’’, or causing it to generate a memory fault or bus error by dereferencing an invalid pointer.

%n can be used to write arbitrary data to potentially carefully-selected addresses. Programmers are therefore strongly advised to never pass untrusted strings as the format argument, as an attacker can put format specifiers in the string to mangle your stack, leading to a possible security hole. This holds true even if the string was built using a function like snprintf(), as the resulting string may still contain user-supplied conversion specifiers for later interpolation by printf(). For this reason, a format argument containing %n is assumed to be untrustworthy if located in writable memory (i.e. memory with protection PROT_WRITE; see mprotect(2)) and any attempt to use such an argument is fatal. Practically, this means that %n is permitted in literal format strings but disallowed in format strings located in normal stack- or heap-allocated memory.

Always use the proper secure idiom:

snprintf(buffer, sizeof(buffer), “%s”, string);

Mitigation

Using snprintf which prevents buffers overflows and returns the minimal size of buffer needed to fit the whole formatted string.

printf() and Uncontrolled format string

Uncontrolled format string is a vulnerability category which can cause information leakage and overwriting of memory. The following functions are also vulnerable: printf, fprintf, sprintf and snprintf. Also look at other functions that take a “format string” as argument.

A malicious user may use the %s and %x format tokens, among others, to print data from the call stack or possibly other locations in memory. One may also write arbitrary data to arbitrary locations using the %n format token, which commands printf() and similar functions to write the number of bytes formatted to an address stored on the stack.

$ gcc -mpreferred-stack-boundary=2 format.c -o FormatString
$ ./FormatString %s
This is a secret!
$

Mitigation

Always hardcode the format string. At least, never let it come directly from any user’s input.