Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

multiple buffer overflow vulnerabilities in lufa #172

Closed
hac425xxx opened this issue Jun 8, 2021 · 3 comments
Closed

multiple buffer overflow vulnerabilities in lufa #172

hac425xxx opened this issue Jun 8, 2021 · 3 comments

Comments

@hac425xxx
Copy link

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
Copy link
Owner

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
Copy link

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 >...

if (ReadMemory_XPROG_Params.Length >= sizeof(ReadBuffer))

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
Copy link
Owner

Quite right @masterxq - will patch now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants