tree 63c66545c52cb4f4f61bd3cee727a4a6f092a709
parent c59e0c1baf5a69d96416eccf8eb208c10aab39e5
author Drew Fisher <zarvox@google.com> 1589993200 +0000
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> 1589993200 +0000

[usb-device] Verify wTotalLength sanity

A malicious USB device could change the value it returns in wTotalLength
when the USB host reads configuration descriptors, because those
descriptors must be requested twice -- once to read only the header of
the descriptor to learn the length, and then once again to retrieve the
entire descriptor into an appropriately-sized buffer.

We already verified that the number of bytes we read back in the second
request matched the length specified in the first, but we did not verify
that the contents of the second request matched those of the first.
Failing to do so would leave a descriptor that claims to be longer than
the buffer allocated to it is, which if not handled very carefully could
lead to out-of-bounds reads.  Indeed, the rest of the code treats
wTotalLength as authoritative.

If a device attempts such trickery, we should reject the device in the
same way we'd reject it if it gave us a short read.

Additionally, we should reject wTotalLength values that are shorter than
the config descriptor header -- they can't possibly be valid.

Test: added new test for this case to usb-device-test that fails before
the change but passes after.

We're grateful to Quarkslab for reporting this vulnerability.

Fixed: 50619
Change-Id: I70caa5c1da47c305fc4bcd32c0d35484ca18a323
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/391569
Reviewed-by: Brian Bosak <bbosak@google.com>
Testability-Review: Drew Fisher <zarvox@google.com>
Commit-Queue: Drew Fisher <zarvox@google.com>