Page MenuHomeFreeBSD

upgt: add some overflow checks
Needs ReviewPublic

Authored by avos on Jul 30 2017, 10:37 PM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 23 2023, 9:58 AM
Unknown Object (File)
Dec 8 2023, 3:35 AM
Unknown Object (File)
Nov 26 2023, 1:52 PM
Unknown Object (File)
Nov 15 2023, 1:48 AM
Unknown Object (File)
Nov 11 2023, 12:15 PM
Unknown Object (File)
Nov 5 2023, 3:25 PM
Unknown Object (File)
Nov 3 2023, 11:53 PM
Unknown Object (File)
Nov 1 2023, 10:29 PM
Subscribers

Details

Summary

In upgt_eeprom_parse():

  • change type of 'option_len' from uint16_t to uint32_t to prevent multiplication overflow;
  • check for 'eeprom_option' overflow (maybe useless?).

In upgt_fw_verify():

  • fix Boot Record Area presence check;
  • reject firmware if 'bra_option_len' overflows.
Test Plan

Untested

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

sys/dev/usb/wlan/if_upgt.c
1382–1383

missing range checks for eeprom_len being valid.

1412

missing range check for header->header1.len being valid.

1447

uint32_t pkglen

1462–1469

if (pkglen > (MCLBYTES - ETHER_ALIGN - IEEE80211_CRC_LEN ))

xxx error;
1470–1471

Remove this KASSERT() - we don't want to panic on corrupt USB data!

1727

Please add checks here for frame_start wrapping around.

1772–1774

The for-loop condition should read: (offset + sizeof(*uc)) <= fw->datasize

Else you might read data which is not inside the BRA.

1779

Same here.

1804

Please also add this check:

if (sizeof(struct upgt_fw_bra_option) > (size_t)(fw->datasize - offset) ||
    (bra_option_len > (size_t)(fw->datasize - offset - sizeof(struct upgt_fw_bra_option)))
sys/dev/usb/wlan/if_upgt.c
2359

IMHO, this code path will never work: typically error handling duplicates USB_ST_TRANSFERRED event with non-zero status code passed to ieee80211_tx_complete().
Here 1) mbuf is not freed - memory leak? 2) data buffer is not moved from active to inactive ring, so Tx will stop.

Turn some KASSERT's into checks + add more size checks.

avos marked 7 inline comments as done.Aug 21 2017, 2:47 AM
avos marked an inline comment as done.Aug 21 2017, 8:13 AM

Add some checks against actlen.

avos marked an inline comment as done.Aug 22 2017, 9:48 PM