Default network.c implementation has fatal security flaw

[GoogleCodeExporter]

The default r152 network.c function network_read() does not check the buffer 
overflow, so if packet larger than UIP_BUFSIZE is sent to device, it will 
overwrite the device memory.

The simple (perhaps not too elegant) fix can be as following:

unsigned int
network_read(void) {
    uint16_t len, totallen;
    char dummy[1];
    int i;

    len = ksz8851BeginPacketRetrieve();
    totallen = len;

    if (len == 0) {
        return 0;
    }

    // Avoid SCADA attack - do not overflow
    if (len > UIP_BUFSIZE)
        len = UIP_BUFSIZE;

    ksz8851RetrievePacketData((uint8_t *)uip_buf, len);

    // fetch reminder of the data to prevent stalling
    for (i=len; i<totallen; i++)
        ksz8851RetrievePacketData((uint8_t *)dummy, 1);

    ksz8851EndPacketRetrieve();

    return len;
}

Original issue reported on code.google.com by andrus.a...@gmail.com on 6 Dec 2011 at 10:17

[GoogleCodeExporter]

Easily repeatable by pinging the device with large packets (ping x.x.x.x -l 
60000)

Original comment by andrus.a...@gmail.com on 6 Dec 2011 at 10:18

[GoogleCodeExporter]

Cool!  Thank YOU!

I don't have a dev board with this chip.  Once I make the change, please check 
out the updated code and test it.

Original comment by qarc...@gmail.com on 7 Dec 2011 at 5:07

• Changed state: Accepted

[GoogleCodeExporter]

Fix committed revision 153.

Please comment the fix is correct once you test it again and I'll close out the 
issue.

Thanks

Original comment by qarc...@gmail.com on 7 Dec 2011 at 5:22

• Changed state: Fixed