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)
Tue, Nov 5, 3:44 AM
Unknown Object (File)
Oct 6 2024, 7:25 AM
Unknown Object (File)
Oct 4 2024, 4:40 PM
Unknown Object (File)
Oct 4 2024, 5:15 AM
Unknown Object (File)
Oct 4 2024, 3:45 AM
Unknown Object (File)
Oct 3 2024, 8:33 AM
Unknown Object (File)
Oct 3 2024, 8:33 AM
Unknown Object (File)
Oct 3 2024, 8:33 AM
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
1384–1385

missing range checks for eeprom_len being valid.

1414

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

1449

uint32_t pkglen

1464–1471

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

xxx error;
1472–1473

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

1738

Please add checks here for frame_start wrapping around.

1791–1793

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

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

1798

Same here.

1823

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
2378

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