libusb
libusb copied to clipboard
libusb
Coverity scan report -- libusb/os/linux_usbfs.c
This is the Defect reported by Coverity Scan for libusb/libusb.
Type: untrusted loop bound Impact: medium Status: New First detected: 16-Sept-2021
** CID 338869: (TAINTED_SCALAR)
/libusb/os/linux_usbfs.c: 690 in parse_config_descriptors()
/libusb/os/linux_usbfs.c: 697 in parse_config_descriptors()
________________________________________________________________________________________________________
*** CID 338869: (TAINTED_SCALAR)
/libusb/os/linux_usbfs.c: 734 in parse_config_descriptors()
728 * with an invalid bLength removed.
729 */
730 uint16_t sysfs_config_len;
731 int offset;
732
733 if (num_configs > 1 && idx < num_configs - 1) {
>>> CID 338869: (TAINTED_SCALAR)
>>> Passing tainted expression "remaining" to "seek_to_next_config", which uses it as a loop boundary.
734 offset = seek_to_next_config(ctx, buffer, remaining);
735 if (offset < 0)
736 return offset;
737 sysfs_config_len = (uint16_t)offset;
738 } else {
739 sysfs_config_len = (uint16_t)remaining;
/libusb/os/linux_usbfs.c: 690 in parse_config_descriptors()
684 device_desc = priv->descriptors;
685 num_configs = device_desc->bNumConfigurations;
686
687 if (num_configs == 0)
688 return 0; /* no configurations? */
689
>>> CID 338869: (TAINTED_SCALAR)
>>> Passing tainted expression "num_configs * 16UL" to "malloc", which uses it as an allocation size.
690 priv->config_descriptors = malloc(num_configs * sizeof(priv->config_descriptors[0]));
691 if (!priv->config_descriptors)
692 return LIBUSB_ERROR_NO_MEM;
693
694 buffer = (uint8_t *)priv->descriptors + LIBUSB_DT_DEVICE_SIZE;
695 remaining = priv->descriptors_len - LIBUSB_DT_DEVICE_SIZE;
/libusb/os/linux_usbfs.c: 697 in parse_config_descriptors()
691 if (!priv->config_descriptors)
692 return LIBUSB_ERROR_NO_MEM;
693
694 buffer = (uint8_t *)priv->descriptors + LIBUSB_DT_DEVICE_SIZE;
695 remaining = priv->descriptors_len - LIBUSB_DT_DEVICE_SIZE;
696
>>> CID 338869: (TAINTED_SCALAR)
>>> Using tainted variable "num_configs" as a loop boundary.
697 for (idx = 0; idx < num_configs; idx++) {
698 struct usbi_configuration_descriptor *config_desc;
699 uint16_t config_len;
700
701 if (remaining < LIBUSB_DT_CONFIG_SIZE) {
702 usbi_err(ctx, "short descriptor read %zu/%d",
/libusb/os/linux_usbfs.c: 734 in parse_config_descriptors()
728 * with an invalid bLength removed.
729 */
730 uint16_t sysfs_config_len;
731 int offset;
732
733 if (num_configs > 1 && idx < num_configs - 1) {
>>> CID 338869: (TAINTED_SCALAR)
>>> Passing tainted expression "remaining" to "seek_to_next_config", which uses it as a loop boundary.
734 offset = seek_to_next_config(ctx, buffer, remaining);
735 if (offset < 0)
736 return offset;
737 sysfs_config_len = (uint16_t)offset;
738 } else {
739 sysfs_config_len = (uint16_t)remaining;
Trace.
675static int parse_config_descriptors(struct libusb_device *dev)
676{
677 struct libusb_context *ctx = DEVICE_CTX(dev);
678 struct linux_device_priv *priv = usbi_get_device_priv(dev);
679 struct usbi_device_descriptor *device_desc;
680 uint8_t idx, num_configs;
681 uint8_t *buffer;
682 size_t remaining;
683
1. tainted_data_downcast: Downcasting priv->descriptors from void * to struct usbi_device_descriptor implies that the data that this pointer points to is tainted.
684 device_desc = priv->descriptors;
685 num_configs = device_desc->bNumConfigurations;
686
2. Condition num_configs == 0, taking false branch.
687 if (num_configs == 0)
688 return 0; /* no configurations? */
689
CID 338869 (#2 of 4): Untrusted loop bound (TAINTED_SCALAR) [select issue]
690 priv->config_descriptors = malloc(num_configs * sizeof(priv->config_descriptors[0]));
3. Condition !priv->config_descriptors, taking false branch.
691 if (!priv->config_descriptors)
692 return LIBUSB_ERROR_NO_MEM;
693
4. var_assign_var: Assigning: buffer = (uint8_t *)priv->descriptors + 18. Both are now tainted.
694 buffer = (uint8_t *)priv->descriptors + LIBUSB_DT_DEVICE_SIZE;
695 remaining = priv->descriptors_len - LIBUSB_DT_DEVICE_SIZE;
696
CID 338869 (#3 of 4): Untrusted loop bound (TAINTED_SCALAR) [select issue]
5. Condition idx < num_configs, taking true branch.
28. Condition idx < num_configs, taking true branch.
47. Condition idx < num_configs, taking true branch.
697 for (idx = 0; idx < num_configs; idx++) {
698 struct usbi_configuration_descriptor *config_desc;
699 uint16_t config_len;
700
6. Condition remaining < 9, taking false branch.
29. Condition remaining < 9, taking false branch.
30. lower_bounds: Checking lower bounds of unsigned scalar remaining by taking the false branch of remaining < 9UL.
48. Condition remaining < 9, taking false branch.
49. lower_bounds: Checking lower bounds of unsigned scalar remaining by taking the false branch of remaining < 9UL.
701 if (remaining < LIBUSB_DT_CONFIG_SIZE) {
702 usbi_err(ctx, "short descriptor read %zu/%d",
703 remaining, LIBUSB_DT_CONFIG_SIZE);
704 return LIBUSB_ERROR_IO;
705 }
706
707 config_desc = (struct usbi_configuration_descriptor *)buffer;
7. Condition config_desc->bDescriptorType != LIBUSB_DT_CONFIG, taking false branch.
31. Condition config_desc->bDescriptorType != LIBUSB_DT_CONFIG, taking false branch.
50. Condition config_desc->bDescriptorType != LIBUSB_DT_CONFIG, taking false branch.
708 if (config_desc->bDescriptorType != LIBUSB_DT_CONFIG) {
709 usbi_err(ctx, "descriptor is not a config desc (type 0x%02x)",
710 config_desc->bDescriptorType);
711 return LIBUSB_ERROR_IO;
8. Condition config_desc->bLength < 9, taking false branch.
32. Condition config_desc->bLength < 9, taking false branch.
51. Condition config_desc->bLength < 9, taking false branch.
712 } else if (config_desc->bLength < LIBUSB_DT_CONFIG_SIZE) {
713 usbi_err(ctx, "invalid descriptor bLength %u",
714 config_desc->bLength);
715 return LIBUSB_ERROR_IO;
716 }
717
718 config_len = libusb_le16_to_cpu(config_desc->wTotalLength);
9. Condition config_len < 9, taking false branch.
33. Condition config_len < 9, taking false branch.
52. Condition config_len < 9, taking false branch.
719 if (config_len < LIBUSB_DT_CONFIG_SIZE) {
720 usbi_err(ctx, "invalid wTotalLength %u", config_len);
721 return LIBUSB_ERROR_IO;
722 }
723
10. Condition priv->sysfs_dir, taking true branch.
34. Condition priv->sysfs_dir, taking true branch.
53. Condition priv->sysfs_dir, taking true branch.
724 if (priv->sysfs_dir) {
725 /*
726 * In sysfs wTotalLength is ignored, instead the kernel returns a
727 * config descriptor with verified bLength fields, with descriptors
728 * with an invalid bLength removed.
729 */
730 uint16_t sysfs_config_len;
731 int offset;
732
11. Condition num_configs > 1, taking true branch.
12. Condition idx < num_configs - 1, taking true branch.
35. Condition num_configs > 1, taking true branch.
36. Condition idx < num_configs - 1, taking false branch.
54. Condition num_configs > 1, taking true branch.
55. Condition idx < num_configs - 1, taking true branch.
733 if (num_configs > 1 && idx < num_configs - 1) {
13. tainted_data_transitive: Call to function seek_to_next_config with tainted argument *buffer returns tainted data. [show details]
14. var_assign: Assigning: offset = seek_to_next_config(ctx, buffer, remaining), which taints offset.
CID 338869 (#1-4 of 4): Untrusted loop bound (TAINTED_SCALAR)
56. tainted_data: Passing tainted expression remaining to seek_to_next_config, which uses it as a loop boundary. [show details]
Ensure that tainted values are properly sanitized, by checking that their values are within a permissible range.
734 offset = seek_to_next_config(ctx, buffer, remaining);
15. Condition offset < 0, taking false branch.
16. lower_bounds: Checking lower bounds of signed scalar offset by taking the false branch of offset < 0.
735 if (offset < 0)
736 return offset;
17. var_assign_var: Assigning: sysfs_config_len = (uint16_t)offset. Both are now tainted.
737 sysfs_config_len = (uint16_t)offset;
18. Falling through to end of if statement.
738 } else {
37. var_assign_var: Assigning: sysfs_config_len = (uint16_t)remaining. Both are now tainted.
739 sysfs_config_len = (uint16_t)remaining;
740 }
741
19. lower_bounds: Casting narrower unsigned sysfs_config_len to wider signed type int effectively tests its lower bound.
20. Condition config_len != sysfs_config_len, taking true branch.
38. lower_bounds: Casting narrower unsigned sysfs_config_len to wider signed type int effectively tests its lower bound.
39. Condition config_len != sysfs_config_len, taking true branch.
742 if (config_len != sysfs_config_len) {
21. lower_bounds: Casting narrower unsigned sysfs_config_len to wider signed type int effectively tests its lower bound.
40. lower_bounds: Casting narrower unsigned sysfs_config_len to wider signed type int effectively tests its lower bound.
743 usbi_warn(ctx, "config length mismatch wTotalLength %u real %u",
744 config_len, sysfs_config_len);
22. var_assign_var: Assigning: config_len = sysfs_config_len. Both are now tainted.
41. var_assign_var: Assigning: config_len = sysfs_config_len. Both are now tainted.
745 config_len = sysfs_config_len;
746 }
23. Falling through to end of if statement.
42. Falling through to end of if statement.
747 } else {
748 /*
749 * In usbfs the config descriptors are wTotalLength bytes apart,
750 * with any short reads from the device appearing as holes in the file.
751 */
752 if (config_len > remaining) {
753 usbi_warn(ctx, "short descriptor read %zu/%u", remaining, config_len);
754 config_len = (uint16_t)remaining;
755 }
756 }
757
24. Condition config_desc->bConfigurationValue == 0, taking true branch.
43. Condition config_desc->bConfigurationValue == 0, taking true branch.
758 if (config_desc->bConfigurationValue == 0)
759 usbi_warn(ctx, "device has configuration 0");
760
761 priv->config_descriptors[idx].desc = config_desc;
762 priv->config_descriptors[idx].actual_len = config_len;
763
25. var_assign_var: Compound assignment involving tainted variable config_len to variable buffer taints buffer.
44. var_assign_var: Compound assignment involving tainted variable config_len to variable buffer taints buffer.
764 buffer += config_len;
26. var_assign_var: Compound assignment involving tainted variable config_len to variable remaining taints remaining.
45. var_assign_var: Compound assignment involving tainted variable config_len to variable remaining taints remaining.
765 remaining -= config_len;
27. Jumping back to the beginning of the loop.
46. Jumping back to the beginning of the loop.
766 }
767
768 return LIBUSB_SUCCESS;
769}
```
I don't understand the problem here. I seems that Coverity complains about casting priv->descriptors from void * to a different type and then accessing that casted data. All information that is derived from that casted void pointer is assumed to be tainted/untrusted. But I don't get why.
Does Coverity give a more detailed explanation about this issue?
No, that is all it gives. We can triage it as not an issue if we think therer is no issue or false positive.
I think therer is no issue. The reasons are as follows：
CID 338869: (TAINTED_SCALAR) Passing tainted expression "remaining" to "seek_to_next_config", which uses it as a loop boundary.
The value of remaining has been checked befor calling seek_to_next_config. if (remaining < LIBUSB_DT_CONFIG_SIZE) { usbi_err(ctx, "short descriptor read %zu/%d", remaining, LIBUSB_DT_CONFIG_SIZE); return LIBUSB_ERROR_IO; }
CID 338869: (TAINTED_SCALAR) Passing tainted expression "num_configs * 16UL" to "malloc", which uses it as an allocation size.
num_configs is 8UL，num_configs * 16UL would not exceed size_t（the input parameters units of malloc.）
CID 338869: (TAINTED_SCALAR) Using tainted variable "num_configs" as a loop boundary.
num_configs is 8UL，is no risk as the exit condition of the loop。
