Re-Inventing the Wheel is Evil

Once I had to debug a program that ran in the Teja application framework for IXP 1200. We experienced random crashes at sites that were using a Walled Garden wireless authentication method.

By another name the Captive Portal consist in a TCP stream hijacker. The intercepted stream is HTTP. The component that handles this functionality simply pretends to be any HTTP destination, analyses the request and sends a crafted HTTP/307 redirection message in which the original URL is encoded as a GET parameter.

The Teja module that implemented the redirection got as an input an Ethernet frame (TCP reassembly was not performed but that was not a practical problem as most people simply go to http://yahoo.com, i.e. use short URLs that fit within one frame).

The data structure that enveloped the Ethernet frame was called Packet and it looked like:

struct Packet {
    char  buffer[1500];
    short start;
    short end;
};

To an astute reader this resembles an skbuff.

The code was parsing the input request and sending back a 307 that looked like "Location: http://auth.host/login.php?dest=OriginalURL"

The response was kept in a 6000-byte buffer which was large enough but here's what they were doing:

memcpy(packet.buffer+packet.end, response, response_len);
packet.end += response_len;

See anything wrong? If response_len>1500 then the memcpy thrashes packet.end, then one adds something to it resulting in a bigger junk. How about the junk in packet.start?

The Packets were malloc'ed [thus living in a pool of objects and having neighbours] so this had the potential (if len>1508) to corrupt the "secret stuff" the GNU allocator puts before each malloc'ed pointer. This will cause a happy crash when you want to free that pointer!

Later on the packet was copied into a hardware buffer that lived in IXP DRAM mapped in the Arm CPU address space like so:

memcpy(pHwBuf->buffer, \
       packet.buffer+packet.start, \
       (packet.end-packet.start));

Here you could copy garbage from a garbage address (packet.buffer+packet.start) [no crash if the memory was mapped] that had a garbage length (packet.end-packet.start) thus thrashing memory left and right in unexpected places.

Murphy's Laws dictate that the memory belonged to other threads of execution and that these threads will crash at another time in a such a way to turn you green.

In order to track that I had to change the Packet like so:

struct Packet {
    unsigned sig1 = 0xdeadbeef; // initialised when
                                // allocating a Packet
    char buffer[1500];
    unsigned sig2 = 0xfeedabba;
    short start;
    short end;
    unsigned sig3 = 0xdeafabba;
};

and check the signatures after each Packet operation. Took me only a month and one very pissed customer to fix this (not my code).

-ulianov

P.S. The Linux skbuff infrastructure comes with a lot of accessory functions that prevent this kind of crap from happening by Oops'ing.