hac425xxx commented 3 years ago

multiple buffer overflow vulnerabilities in lufa

Stack Overflow in CCID_Task

path

Demos/Device/LowLevel/CCID/CCID.c

it first receive data to USB_CCID_BulkMessage_Header_t

        USB_CCID_BulkMessage_Header_t CCIDHeader;
        CCIDHeader.MessageType = Endpoint_Read_8();
        CCIDHeader.Length      = Endpoint_Read_32_LE();
        CCIDHeader.Slot        = Endpoint_Read_8();
        CCIDHeader.Seq         = Endpoint_Read_8();

then in CCID_PC_to_RDR_XfrBlock , it could read CCIDHeader.Length data to RequestBuffer

case CCID_PC_to_RDR_XfrBlock:
{
    uint8_t  Bwi            = Endpoint_Read_8();
    uint16_t LevelParameter = Endpoint_Read_16_LE();

    (void)Bwi;
    (void)LevelParameter;

    // overflow!!!
    Endpoint_Read_Stream_LE(RequestBuffer, CCIDHeader.Length * sizeof(uint8_t), NULL);

if CCIDHeader.Length > sizeof(RequestBuffer), it could overflow.

buffer overflow in EVENT_USB_Device_ControlRequest

path

Demos/Device/LowLevel/RNDISEthernet/RNDISEthernet.c

code

void EVENT_USB_Device_ControlRequest(void)
{

    switch (USB_ControlRequest.bRequest)
    {
        case RNDIS_REQ_SendEncapsulatedCommand:

        // overflow !!!
        Endpoint_Read_Control_Stream_LE(RNDISMessageBuffer, USB_ControlRequest.wLength);

if USB_ControlRequest.wLength > sizeof(RNDISMessageBuffer), it could overflow.

out of bound access in IP_ProcessIPPacket

path:

Demos/Device/LowLevel/RNDISEthernet/Lib/IP.c

code

IP_Header_t* IPHeaderIN  = (IP_Header_t*)InDataStart;

uint16_t HeaderLengthBytes = (IPHeaderIN->HeaderLength * sizeof(uint32_t));
switch (IPHeaderIN->Protocol)
{
    case PROTOCOL_ICMP:
        // vuln !!!
        RetSize = ICMP_ProcessICMPPacket(&((uint8_t*)InDataStart)[HeaderLengthBytes],
                                            &((uint8_t*)OutDataStart)[sizeof(IP_Header_t)]);

if IPHeaderIN->HeaderLength > the size of InDataStart, it could out of bound!

buffer overflow in RNDIS_Device_ProcessControlRequest

path:

LUFA/Drivers/USB/Class/Device/RNDISClassDevice.c

code

    switch (USB_ControlRequest.bRequest)
    {
        case RNDIS_REQ_SendEncapsulatedCommand:

                // overflow !!!
                Endpoint_Read_Control_Stream_LE(RNDISInterfaceInfo->Config.MessageBuffer, USB_ControlRequest.wLength);

if USB_ControlRequest.wLength > the size of MessageBuffer, it could buffer overflow!

stack overflow in ISPProtocol_SPIMulti

path:

Projects/AVRISP-MKII/Lib/ISP/ISPProtocol.c

code

/** Handler for the CMD_SPI_MULTI command, writing and reading arbitrary SPI data to and from the attached device. */
void ISPProtocol_SPIMulti(void)
{
    struct
    {
        uint8_t TxBytes;
        uint8_t RxBytes;
        uint8_t RxStartAddr;
        uint8_t TxData[255];
    } SPI_Multi_Params;

    Endpoint_Read_Stream_LE(&SPI_Multi_Params, (sizeof(SPI_Multi_Params) - sizeof(SPI_Multi_Params.TxData)), NULL);

    // buffer overflow !!!
    Endpoint_Read_Stream_LE(&SPI_Multi_Params.TxData, SPI_Multi_Params.TxBytes, NULL);

if SPI_Multi_Params.TxBytes > the size of SPI_Multi_Params.TxData, it could buffer overflow!

stack overflow in XPROGProtocol_WriteMemory

path:

Projects/AVRISP-MKII/Lib/XPROG/XPROGProtocol.c

code

static void XPROGProtocol_WriteMemory(void)
{
    uint8_t ReturnStatus = XPROG_ERR_OK;

    struct
    {
        uint8_t  MemoryType;
        uint8_t  PageMode;
        uint32_t Address;
        uint16_t Length;
        uint8_t  ProgData[256];
    } WriteMemory_XPROG_Params;

    Endpoint_Read_Stream_LE(&WriteMemory_XPROG_Params, (sizeof(WriteMemory_XPROG_Params) -
                                                        sizeof(WriteMemory_XPROG_Params).ProgData), NULL);
    WriteMemory_XPROG_Params.Address = SwapEndian_32(WriteMemory_XPROG_Params.Address);
    WriteMemory_XPROG_Params.Length  = SwapEndian_16(WriteMemory_XPROG_Params.Length);

    // overflow!!!
    Endpoint_Read_Stream_LE(&WriteMemory_XPROG_Params.ProgData, WriteMemory_XPROG_Params.Length, NULL);

if WriteMemory_XPROG_Params.Length > the size of WriteMemory_XPROG_Params.ProgData, it could buffer overflow!

stack overflow in XPROGProtocol_ReadMemory

path:

Projects/AVRISP-MKII/Lib/XPROG/XPROGProtocol.c

code


static void XPROGProtocol_ReadMemory(void)
{
    uint8_t ReturnStatus = XPROG_ERR_OK;

    struct
    {
        uint8_t  MemoryType;
        uint32_t Address;
        uint16_t Length;
    } ReadMemory_XPROG_Params;

    Endpoint_Read_Stream_LE(&ReadMemory_XPROG_Params, sizeof(ReadMemory_XPROG_Params), NULL);
    ReadMemory_XPROG_Params.Address = SwapEndian_32(ReadMemory_XPROG_Params.Address);
    ReadMemory_XPROG_Params.Length  = SwapEndian_16(ReadMemory_XPROG_Params.Length);

    uint8_t ReadBuffer[256];

    // overflow!!!
    if (!(XMEGANVM_ReadMemory(ReadMemory_XPROG_Params.Address, ReadBuffer, ReadMemory_XPROG_Params.Length)))

if ReadMemory_XPROG_Params.Length > the size of ReadBuffer, it could buffer overflow!

abcminiuser commented 3 years ago

Thanks for the report - I believe I've now addresses these in the latest commits. I've ended up removing the cruddy TCP/IP code from the examples entirely, as it was more of a learning exercise for myself over a decade ago than practical code for others to use.

I've added the missing length checks in the other cases.

masterxq commented 2 years ago

The fix is broken. You checking if requested length is >= buffer size. This will reject requests with == buffer size. What is a normal buffer size and should be allowed! Try to reject only if >... https://github.com/abcminiuser/lufa/blob/336f3570a7e6c566fe62084414eb0464ba0ccf40/Projects/AVRISP-MKII/Lib/XPROG/XPROGProtocol.c#L352

It's also part of my PR... But i think it should be repaired instantly for allowing to use XPROGProtocol without stall again^^

Regards

abcminiuser commented 2 years ago

Quite right @masterxq - will patch now.

abcminiuser / lufa