PATCH: libhttpd crash on invalid HTTP headers

[anonymous]

In libhttpd/api.c, line 532 you write:

cp = index(buf,':') + 2; if(cp) {

This is a really nasty bug: if index() returns NULL, cp becomes 2 and if(cp) only fails if index returns -2. This will never happen. Therefore you should write something like this:

cp = index(buf,':'); if(cp) {

cp += 2;

Greetings, CK

[anonymous]

What about this bug? It's really critical. Had non-reproducable crashes and it caused me about 2 hours to find it.

[anonymous]

This bug caused our wifidog gateway (>1000 users) to crash every 5 to 20 minutes (non-reproducable, too). After patching, the gateway works like a charm.

[wichert]

Your change does not make sense: Since the strncasecmp() call returned a match it is impossible for index to return anything but a sensible index. If it returns NULL the only possible reason is that your machine has bad memory which happened to flip a bit.

In fact since we know that strncasecmp returned a match we already know the exact index of the colon, so the whole test is senseless.

What would make sense is to test for *cp instead, ie test if there is an actual value specified in the HTTP host header. Could it be that you have incoming requests that contain an empty Host header?

[anonymous]

I dunno. It's not my machine, I only searched for the bug. I didn't look for the sense of the code, I saw what the error was: index() (sic! not strncasecmp()) returned NULL and therefore cp was 2 and passed if(cp). That led to memory access at address 0 causing a segfault.

[anonymous]

Memory access at address 2! not 0!

[jean-philippe.menil@…]

I've submitted core debug on this ticket (527). The workaround fix the problem, who appear only with a lot of users.

[benoitg]

Fixed in [1423]

[jean-philippe.menil@…]

Hi,
i reopen the ticket due to the following bug:
Core was generated by `/usr/bin/wifidog -s -c /etc/wifidog/XX.conf -w /tmp/wdctl-XX.sock'.
Program terminated with signal 11, Segmentation fault.
[New process 8650]
[New process 8651]
[New process 8652]
[New process 8656]
[New process 8654]
[New process 10017]
[New process 8657]
[New process 10019]
[New process 8659]
[New process 8655]
[New process 8658]
[New process 10018]
[New process 8653]
[New process 9906]
#0 0x00007f1f30def82c in httpdReadRequest (server=0x143a170, r=0x7f1f200072d0) at api.c:493.[[BR]] 493 if (strncmp(cp,"Basic ", 6) != 0)
(gdb) where
#0 0x00007f1f30def82c in httpdReadRequest (server=0x143a170, r=0x7f1f200072d0) at api.c:493[[BR]] #1 0x000000000040b41a in thread_httpd (args=<value optimized out>) at httpd_thread.c:61[[BR]] #2 0x00007f1f309bd73a in pthread_create@@GLIBC_2.2.5 () from /lib/libpthread.so.0
#3 0x0000000000000000 in ?? ()

I can't reproduce it, as i can't reproduce the bug above.
It happens rarely, but it crash the gateway.

[gbastien]

Patch for second part of the bug

[gbastien]

The fix applied for this bug was done in a "if" block starting with

if (strncasecmp(buf,"Host: ",6) == 0) (line 533)

The line mentioned here is from a different block with the same pattern starting with

if (strncasecmp(buf,"Authorization: ",15) == 0) (line 493)

A quick guess would be to do the same fix to that other block of code.

Here's a patch (untested for now)