Skip to content

Commit

Permalink
WIP: buffer overread
Browse files Browse the repository at this point in the history
The first iteration of this loop was safe because the beginning of the function checked that `size` is at least LIBUSB_DT_CONFIG_SIZE (9) bytes long.

But for subsequent iterations, it's unclear...
  • Loading branch information
seanm committed Feb 7, 2024
1 parent cb78999 commit 83942ce
Showing 1 changed file with 10 additions and 7 deletions.
17 changes: 10 additions & 7 deletions libusb/descriptor.c
Original file line number Diff line number Diff line change
Expand Up @@ -1245,7 +1245,7 @@ static int parse_iad_array(struct libusb_context *ctx,
while (consumed < size) {
header.bLength = buf[0];
header.bDescriptorType = buf[1];
if (header.bLength < 2) {
if (header.bLength < sizeof(struct usbi_descriptor_header)) {
usbi_err(ctx, "invalid descriptor bLength %d",
header.bLength);
return LIBUSB_ERROR_IO;
Expand All @@ -1270,12 +1270,12 @@ static int parse_iad_array(struct libusb_context *ctx,
iad_array->iad = iad;

// Second pass: Iterate through desc list, fill IAD structures
consumed = 0;
int remaining = size;
i = 0;
while (consumed < size) {
do {
header.bLength = buffer[0];
header.bDescriptorType = buffer[1];
if (header.bDescriptorType == LIBUSB_DT_INTERFACE_ASSOCIATION) {
if (header.bDescriptorType == LIBUSB_DT_INTERFACE_ASSOCIATION && (remaining >= 8)) {
iad[i].bLength = buffer[0];
iad[i].bDescriptorType = buffer[1];
iad[i].bFirstInterface = buffer[2];
Expand All @@ -1286,10 +1286,13 @@ static int parse_iad_array(struct libusb_context *ctx,
iad[i].iFunction = buffer[7];
i++;
}


remaining -= header.bLength;
if (remaining < 2) {
break;
}
buffer += header.bLength;
consumed += header.bLength;
}
} while (1);
}

return LIBUSB_SUCCESS;
Expand Down

0 comments on commit 83942ce

Please sign in to comment.