Reactos

[USBSTOR] Fix PdoHandleQueryInstanceId and increase serial number descriptor size to MAXIMUM_USB_STRING_LENGTH (#6413)

Serial number on some USB devices might exceed the number of 100 characters
(e.g. 120 characters on "SanDisk Ultra 3.2Gen1" pendrive) and cause buffer
overflow, resulting in usbstor.sys crash.

- Use pool allocation for instance ID generation.
Fixes stack overflow on USB storage devices with large serial number.
- Print the LUN number as a hexadecimal, not as a character.
- Verify the serial number descriptor before using it.
- Increase the max descriptor size for serial number to
MAXIMUM_USB_STRING_LENGTH. This fixes serial number string truncation.

Based on suggestions by disean and ThFabba.

CORE-17625

authored by

Adam Słaboń and committed by
GitHub
20efea8f 398201dc

+40 -16
+1 -1
drivers/usb/usbstor/descriptor.c
··· 118 118 if (DeviceExtension->DeviceDescriptor->iSerialNumber) 119 119 { 120 120 // get serial number 121 - Status = USBSTOR_GetDescriptor(DeviceExtension->LowerDeviceObject, USB_STRING_DESCRIPTOR_TYPE, 100 * sizeof(WCHAR), DeviceExtension->DeviceDescriptor->iSerialNumber, 0x0409, (PVOID*)&DeviceExtension->SerialNumber); 121 + Status = USBSTOR_GetDescriptor(DeviceExtension->LowerDeviceObject, USB_STRING_DESCRIPTOR_TYPE, MAXIMUM_USB_STRING_LENGTH, DeviceExtension->DeviceDescriptor->iSerialNumber, 0x0409, (PVOID*)&DeviceExtension->SerialNumber); 122 122 if (!NT_SUCCESS(Status)) 123 123 { 124 124 FreeItem(DeviceExtension->DeviceDescriptor);
+38 -15
drivers/usb/usbstor/pdo.c
··· 437 437 { 438 438 PPDO_DEVICE_EXTENSION PDODeviceExtension; 439 439 PFDO_DEVICE_EXTENSION FDODeviceExtension; 440 - WCHAR Buffer[100]; 441 - ULONG Length; 440 + PUSB_STRING_DESCRIPTOR Descriptor; 441 + ULONG CharCount; 442 442 LPWSTR InstanceId; 443 + NTSTATUS Status; 443 444 444 - PDODeviceExtension = (PPDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension; 445 - FDODeviceExtension = (PFDO_DEVICE_EXTENSION)PDODeviceExtension->LowerDeviceObject->DeviceExtension; 445 + PDODeviceExtension = DeviceObject->DeviceExtension; 446 + FDODeviceExtension = PDODeviceExtension->LowerDeviceObject->DeviceExtension; 446 447 447 - // format instance id 448 - if (FDODeviceExtension->SerialNumber) 448 + Descriptor = FDODeviceExtension->SerialNumber; 449 + if (Descriptor && (Descriptor->bLength >= sizeof(USB_COMMON_DESCRIPTOR) + sizeof(WCHAR))) 449 450 { 450 - // using serial number from device 451 - swprintf(Buffer, L"%s&%c", FDODeviceExtension->SerialNumber->bString, PDODeviceExtension->LUN); 451 + /* Format the serial number descriptor only if supported by the device */ 452 + CharCount = (Descriptor->bLength - sizeof(USB_COMMON_DESCRIPTOR)) / sizeof(WCHAR) + 453 + (sizeof("&") - 1) + 454 + (sizeof("F") - 1) + // LUN: 1 char (MAX_LUN) 455 + sizeof(ANSI_NULL); 452 456 } 453 457 else 454 458 { 455 - // use instance count and LUN 456 - swprintf(Buffer, L"%04lu&%c", FDODeviceExtension->InstanceCount, PDODeviceExtension->LUN); 459 + /* Use the instance count and LUN as a fallback */ 460 + CharCount = (sizeof("99999999") - 1) + // Instance Count: 8 chars 461 + (sizeof("&") - 1) + 462 + (sizeof("F") - 1) + // LUN: 1 char (MAX_LUN) 463 + sizeof(ANSI_NULL); 457 464 } 458 465 459 - Length = wcslen(Buffer) + 1; 460 - 461 - InstanceId = ExAllocatePoolWithTag(PagedPool, Length * sizeof(WCHAR), USB_STOR_TAG); 466 + InstanceId = ExAllocatePoolUninitialized(PagedPool, CharCount * sizeof(WCHAR), USB_STOR_TAG); 462 467 if (!InstanceId) 463 468 { 464 469 Irp->IoStatus.Information = 0; 465 470 return STATUS_INSUFFICIENT_RESOURCES; 466 471 } 467 472 468 - wcscpy(InstanceId, Buffer); 473 + if (Descriptor && (Descriptor->bLength >= sizeof(USB_COMMON_DESCRIPTOR) + sizeof(WCHAR))) 474 + { 475 + Status = RtlStringCchPrintfW(InstanceId, 476 + CharCount, 477 + L"%s&%x", 478 + Descriptor->bString, 479 + PDODeviceExtension->LUN); 480 + } 481 + else 482 + { 483 + Status = RtlStringCchPrintfW(InstanceId, 484 + CharCount, 485 + L"%04lu&%x", 486 + FDODeviceExtension->InstanceCount, 487 + PDODeviceExtension->LUN); 488 + } 469 489 470 - DPRINT("USBSTOR_PdoHandleQueryInstanceId %S\n", InstanceId); 490 + /* This should not happen */ 491 + ASSERT(NT_SUCCESS(Status)); 492 + 493 + DPRINT("USBSTOR_PdoHandleQueryInstanceId '%S'\n", InstanceId); 471 494 472 495 Irp->IoStatus.Information = (ULONG_PTR)InstanceId; 473 496 return STATUS_SUCCESS;
+1
drivers/usb/usbstor/usbstor.h
··· 2 2 #define _USBSTOR_H_ 3 3 4 4 #include <wdm.h> 5 + #include <ntstrsafe.h> 5 6 #include <usbdi.h> 6 7 #include <usbbusif.h> 7 8 #include <usbdlib.h>