I spotted a vulnerability in Icecast, the open source streaming media server maintained by the Xiph.org Foundation. Attackers could craft HTTP headers that would overwrite the server’s stack contents, leading to remote code execution. Since Icecast is commonly used to host internet radio stations, a motivated attacker could potentially take a station off air.

Icecast servers running versions 2.4.0 to 2.4.3 and using URL authentication are affected, and should be upgraded to version 2.4.4 as soon as possible.

The vulnerability has been assigned the identifier CVE-2018-18820.

snprintf: a safer version of sprintf?

At this point, it’s well known that sprintf is unsafe, since it provides no protection against buffer overflow. It’s not unusual to see documentation that points users to snprintf as a safer version, since it will truncate the output if the buffer is too small. But what people often don’t realize is that, when it truncates, snprintf does not return the number of bytes it wrote. In fact, it returns the number of bytes it would have written if the output buffer were big enough. Worse still, it can’t do anything to protect you against buffer overflow if you provide a size argument that’s larger than the actual size of the buffer.

Here’s the vulnerable code from Icecast, where it loops over HTTP headers from a user request and copies them into a buffer, building the body of a POST request to send to an authentication server:

post_offset += snprintf(post + post_offset,
                        sizeof(post) - post_offset,
                        "&%s%s=%s",
                        url->prefix_headers ? url->prefix_headers : "",
                        cur_header, header_valesc);

Let’s look at a slightly simplified version of this code:

post_offset += snprintf(post + post_offset,
                        sizeof(post) - post_offset,
                        "%s",
                        cur_header);

Let’s consider a situation where sizeof(post) is 10, and 8 bytes have already been written (i.e., post_offset is 8). What happens when the next header to be copied is "baz"?

The output gets truncated, but post_offset gets incremented beyond the end of the buffer:

Now let’s imagine there’s another header to be copied, with contents "AAAAA...". We are in an interesting position: the size argument to snprintf is sizeof(post) - post_offset, which will underflow to become an enormously large number. The result is that subsequent calls to snprintf can effectively write as much data as they want. That data will be written to post + post_offset, somewhere beyond the end of the post buffer, and will overwrite other contents of the stack.

This means we can send one long HTTP header that will get truncated, but whose length will allow us to position post_offset anywhere in the stack we choose. Then we can send a second HTTP header whose contents will be written to that location.

One difficulty for an attacker is that some sanitization of the headers is performed before they are copied with snprintf, so they are somewhat limited in what data they can write to the stack. My proof-of-concept exploit only caused a segfault in the server process— effectively a denial-of-service attack—but I suspect a sufficiently motivated and clever attacker would be able to upgrade this attack to achieve full-blown remote code execution.

The fix

The folks at Xiph patched the bug quickly, and the fix is pretty simple. It simply checks the return value from snprintf, and, if it causes post_offset to point beyond the end of the buffer, it logs an error and exits the loop.

LGTM: the automated vulnerability finder

We automatically analyze thousands of open source projects on LGTM, and one of our standard queries produces alerts for suspicious uses of snprintf that seem to match this pattern. That is, where the size argument is derived from the return value of a previous call to snprintf.

My coworker Kevin Backhouse has written in the past about how this query caught a vulnerability in rsyslog, and this vulnerability in Icecast was similar. The alerts were hidden in plain sight, simply waiting for a human to notice them. If you manage or contribute to an open source project, I encourage you to enable GitHub code scanning with CodeQL so you can can notice bugs like this before they get merged into your codebase.

Disclosure timeline