Add missing sanity checks (#727)

Based on #696

- Add missing checks for zero/null arguments;
- Better management of error strings in case of failures;

---------

Co-authored-by: Megamouse <studienricky89@googlemail.com>

Author: Youw

libusb/hid.c

@@ -139,6 +139,9 @@ static int return_data(hid_device *dev, unsigned char *data, size_t length);
 static hid_device *new_hid_device(void)
 {
 	hid_device *dev = (hid_device*) calloc(1, sizeof(hid_device));
+	if (!dev)
+		return NULL;
+
 	dev->blocking = 1;
 
 	hidapi_thread_state_init(&dev->thread_state);

linux/hid.c

@@ -402,8 +402,8 @@ static int get_next_hid_usage(const __u8 *report_descriptor, __u32 size, struct
 	/* If no top-level application collection is found and usage page/usage pair is found, pair is valid
 	   https://docs.microsoft.com/en-us/windows-hardware/drivers/hid/top-level-collections */
 	if (initial && usage_found && ctx->usage_page_found) {
-			*usage_page = ctx->usage_page;
-			return 0; /* success */
+		*usage_page = ctx->usage_page;
+		return 0; /* success */
 	}
 
 	return 1; /* finished processing */
@@ -448,6 +448,8 @@ static int get_hid_report_descriptor_from_sysfs(const char *sysfs_path, struct h
 	/* Construct <sysfs_path>/device/report_descriptor */
 	size_t rpt_path_len = strlen(sysfs_path) + 25 + 1;
 	char* rpt_path = (char*) calloc(1, rpt_path_len);
+	if (!rpt_path)
+		return -1;
 	snprintf(rpt_path, rpt_path_len, "%s/device/report_descriptor", sysfs_path);
 
 	res = get_hid_report_descriptor(rpt_path, rpt_desc);
@@ -784,7 +786,14 @@ static struct hid_device_info * create_device_info_for_device(struct udev_device
 	}
 
 	/* Usage Page and Usage */
-	result = get_hid_report_descriptor_from_sysfs(sysfs_path, &report_desc);
+
+	if (sysfs_path) {
+		result = get_hid_report_descriptor_from_sysfs(sysfs_path, &report_desc);
+	}
+	else {
+		result = -1;
+	}
+
 	if (result >= 0) {
 		unsigned short page = 0, usage = 0;
 		struct hid_usage_iterator usage_iterator;
@@ -809,7 +818,7 @@ static struct hid_device_info * create_device_info_for_device(struct udev_device
 			struct hid_device_info *prev_dev = cur_dev;
 
 			if (!tmp)
-				continue;
+				break;
 			cur_dev->next = tmp;
 			cur_dev = tmp;
 
@@ -854,6 +863,7 @@ static struct hid_device_info * create_device_info_for_hid_device(hid_device *de
 	/* Create the udev object */
 	udev = udev_new();
 	if (!udev) {
+		errno = ENOMEM;
 		register_device_error(dev, "Couldn't create udev context");
 		return NULL;
 	}
@@ -866,6 +876,7 @@ static struct hid_device_info * create_device_info_for_hid_device(hid_device *de
 
 	if (!root) {
 		/* TODO: have a better error reporting via create_device_info_for_device */
+		errno = EIO;
 		register_device_error(dev, "Couldn't create hid_device_info");
 	}
 
@@ -1061,6 +1072,7 @@ hid_device * HID_API_EXPORT hid_open_path(const char *path)
 
 	dev = new_hid_device();
 	if (!dev) {
+		errno = ENOMEM;
 		register_global_error("Couldn't allocate memory");
 		return NULL;
 	}
@@ -1073,8 +1085,8 @@ hid_device * HID_API_EXPORT hid_open_path(const char *path)
 		/* Make sure this is a HIDRAW device - responds to HIDIOCGRDESCSIZE */
 		res = ioctl(dev->device_handle, HIDIOCGRDESCSIZE, &desc_size);
 		if (res < 0) {
-			hid_close(dev);
 			register_global_error_format("ioctl(GRDESCSIZE) error for '%s', not a HIDRAW device?: %s", path, strerror(errno));
+			hid_close(dev);
 			return NULL;
 		}
 
@@ -1095,7 +1107,7 @@ int HID_API_EXPORT hid_write(hid_device *dev, const unsigned char *data, size_t
 
 	if (!data || (length == 0)) {
 		errno = EINVAL;
-		register_device_error(dev, strerror(errno));
+		register_device_error(dev, "Zero buffer/length");
 		return -1;
 	}
 
@@ -1109,6 +1121,12 @@ int HID_API_EXPORT hid_write(hid_device *dev, const unsigned char *data, size_t
 
 int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t length, int milliseconds)
 {
+	if (!data || (length == 0)) {
+		errno = EINVAL;
+		register_error_str(&dev->last_read_error_str, "Zero buffer/length");
+		return -1;
+	}
+
 	/* Set device error to none */
 	register_error_str(&dev->last_read_error_str, NULL);
 
@@ -1142,6 +1160,7 @@ int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t
 			   indicate a device disconnection. */
 			if (fds.revents & (POLLERR | POLLHUP | POLLNVAL)) {
 				// We cannot use strerror() here as no -1 was returned from poll().
+				errno = EIO;
 				register_error_str(&dev->last_read_error_str, "hid_read_timeout: unexpected poll error (device disconnected)");
 				return -1;
 			}
@@ -1186,6 +1205,12 @@ int HID_API_EXPORT hid_send_feature_report(hid_device *dev, const unsigned char
 {
 	int res;
 
+	if (!data || (length == 0)) {
+		errno = EINVAL;
+		register_device_error(dev, "Zero buffer/length");
+		return -1;
+	}
+
 	register_device_error(dev, NULL);
 
 	res = ioctl(dev->device_handle, HIDIOCSFEATURE(length), data);
@@ -1199,6 +1224,12 @@ int HID_API_EXPORT hid_get_feature_report(hid_device *dev, unsigned char *data,
 {
 	int res;
 
+	if (!data || (length == 0)) {
+		errno = EINVAL;
+		register_device_error(dev, "Zero buffer/length");
+		return -1;
+	}
+
 	register_device_error(dev, NULL);
 
 	res = ioctl(dev->device_handle, HIDIOCGFEATURE(length), data);
@@ -1212,6 +1243,12 @@ int HID_API_EXPORT HID_API_CALL hid_send_output_report(hid_device *dev, const un
 {
 	int res;
 
+	if (!data || (length == 0)) {
+		errno = EINVAL;
+		register_device_error(dev, "Zero buffer/length");
+		return -1;
+	}
+
 	register_device_error(dev, NULL);
 
 	res = ioctl(dev->device_handle, HIDIOCSOUTPUT(length), data);
@@ -1225,6 +1262,12 @@ int HID_API_EXPORT HID_API_CALL hid_get_input_report(hid_device *dev, unsigned c
 {
 	int res;
 
+	if (!data || (length == 0)) {
+		errno = EINVAL;
+		register_device_error(dev, "Zero buffer/length");
+		return -1;
+	}
+
 	register_device_error(dev, NULL);
 
 	res = ioctl(dev->device_handle, HIDIOCGINPUT(length), data);
@@ -1253,6 +1296,7 @@ void HID_API_EXPORT hid_close(hid_device *dev)
 int HID_API_EXPORT_CALL hid_get_manufacturer_string(hid_device *dev, wchar_t *string, size_t maxlen)
 {
 	if (!string || !maxlen) {
+		errno = EINVAL;
 		register_device_error(dev, "Zero buffer/length");
 		return -1;
 	}
@@ -1277,6 +1321,7 @@ int HID_API_EXPORT_CALL hid_get_manufacturer_string(hid_device *dev, wchar_t *st
 int HID_API_EXPORT_CALL hid_get_product_string(hid_device *dev, wchar_t *string, size_t maxlen)
 {
 	if (!string || !maxlen) {
+		errno = EINVAL;
 		register_device_error(dev, "Zero buffer/length");
 		return -1;
 	}
@@ -1301,6 +1346,7 @@ int HID_API_EXPORT_CALL hid_get_product_string(hid_device *dev, wchar_t *string,
 int HID_API_EXPORT_CALL hid_get_serial_number_string(hid_device *dev, wchar_t *string, size_t maxlen)
 {
 	if (!string || !maxlen) {
+		errno = EINVAL;
 		register_device_error(dev, "Zero buffer/length");
 		return -1;
 	}
@@ -1324,7 +1370,10 @@ int HID_API_EXPORT_CALL hid_get_serial_number_string(hid_device *dev, wchar_t *s
 
 
 HID_API_EXPORT struct hid_device_info *HID_API_CALL hid_get_device_info(hid_device *dev) {
-	if (!dev->device_info) {
+	if (dev->device_info) {
+		register_device_error(dev, NULL);
+	}
+	else {
 		// Lazy initialize device_info
 		dev->device_info = create_device_info_for_hid_device(dev);
 	}
@@ -1339,6 +1388,7 @@ int HID_API_EXPORT_CALL hid_get_indexed_string(hid_device *dev, int string_index
 	(void)string;
 	(void)maxlen;
 
+	errno = ENOSYS;
 	register_device_error(dev, "hid_get_indexed_string: not supported by hidraw");
 
 	return -1;
@@ -1348,6 +1398,15 @@ int HID_API_EXPORT_CALL hid_get_indexed_string(hid_device *dev, int string_index
 int HID_API_EXPORT_CALL hid_get_report_descriptor(hid_device *dev, unsigned char *buf, size_t buf_size)
 {
 	struct hidraw_report_descriptor rpt_desc;
+
+	if (!buf || !buf_size) {
+		errno = EINVAL;
+		register_device_error(dev, "Zero buffer/length");
+		return -1;
+	}
+
+	register_device_error(dev, NULL);
+
 	int res = get_hid_report_descriptor_from_hidraw(dev, &rpt_desc);
 	if (res < 0) {
 		/* error already registered */

mac/hid.c

@@ -1105,13 +1105,13 @@ static int set_report(hid_device *dev, IOHIDReportType type, const unsigned char
 	IOReturn res;
 	unsigned char report_id;
 
-	register_device_error(dev, NULL);
-
 	if (!data || (length == 0)) {
-		register_device_error(dev, strerror(EINVAL));
+		register_device_error(dev, "Zero buffer/length");
 		return -1;
 	}
 
+	register_device_error(dev, NULL);
+
 	report_id = data[0];
 
 	if (report_id == 0x0) {
@@ -1145,10 +1145,17 @@ static int get_report(hid_device *dev, IOHIDReportType type, unsigned char *data
 	unsigned char *report = data;
 	CFIndex report_length = length;
 	IOReturn res = kIOReturnSuccess;
-	const unsigned char report_id = data[0];
+	unsigned char report_id;
+
+	if (!data || (length == 0)) {
+		register_device_error(dev, "Zero buffer/length");
+		return -1;
+	}
 
 	register_device_error(dev, NULL);
 
+	report_id = data[0];
+
 	if (report_id == 0x0) {
 		/* Not using numbered Reports.
 		   Don't send the report number. */
@@ -1240,13 +1247,17 @@ static int cond_timedwait(hid_device *dev, pthread_cond_t *cond, pthread_mutex_t
 	}
 
 	return 0;
-
 }
 
 int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t length, int milliseconds)
 {
 	int bytes_read = -1;
 
+	if (!data || (length == 0)) {
+		register_error_str(&dev->last_read_error_str, "Zero buffer/length");
+		return -1;
+	}
+
 	register_error_str(&dev->last_read_error_str, NULL);
 
 	/* Lock the access to the report list. */
@@ -1478,7 +1489,10 @@ int HID_API_EXPORT_CALL hid_get_serial_number_string(hid_device *dev, wchar_t *s
 }
 
 HID_API_EXPORT struct hid_device_info *HID_API_CALL hid_get_device_info(hid_device *dev) {
-	if (!dev->device_info) {
+	if (dev->device_info) {
+		register_device_error(dev, NULL);
+	}
+	else {
 		dev->device_info = create_device_info(dev->device_handle);
 		if (!dev->device_info) {
 			register_device_error(dev, "Failed to create hid_device_info");
@@ -1501,6 +1515,13 @@ int HID_API_EXPORT_CALL hid_get_indexed_string(hid_device *dev, int string_index
 
 int HID_API_EXPORT_CALL hid_darwin_get_location_id(hid_device *dev, uint32_t *location_id)
 {
+	if (!location_id) {
+		register_device_error(dev, "Location ID is NULL");
+		return -1;
+	}
+
+	register_device_error(dev, NULL);
+
 	int res = get_int_property(dev->device_handle, CFSTR(kIOHIDLocationIDKey));
 	if (res != 0) {
 		*location_id = (uint32_t) res;
@@ -1523,23 +1544,27 @@ int HID_API_EXPORT_CALL hid_darwin_get_open_exclusive(void)
 
 int HID_API_EXPORT_CALL hid_darwin_is_device_open_exclusive(hid_device *dev)
 {
-	if (!dev)
-		return -1;
-
 	return (dev->open_options == kIOHIDOptionsTypeSeizeDevice) ? 1 : 0;
 }
 
 int HID_API_EXPORT_CALL hid_get_report_descriptor(hid_device *dev, unsigned char *buf, size_t buf_size)
 {
+	if (!buf || !buf_size) {
+		register_device_error(dev, "Zero buffer/length");
+		return -1;
+	}
+
+	register_device_error(dev, NULL);
+
 	CFTypeRef ref = IOHIDDeviceGetProperty(dev->device_handle, CFSTR(kIOHIDReportDescriptorKey));
 	if (ref != NULL && CFGetTypeID(ref) == CFDataGetTypeID()) {
 		CFDataRef report_descriptor = (CFDataRef) ref;
 		const UInt8 *descriptor_buf = CFDataGetBytePtr(report_descriptor);
-		CFIndex descriptor_buf_len = CFDataGetLength(report_descriptor);
+		const CFIndex descriptor_buf_len = CFDataGetLength(report_descriptor);
 		size_t copy_len = (size_t) descriptor_buf_len;
 
 		if (descriptor_buf == NULL || descriptor_buf_len < 0) {
-			register_device_error(dev, "Zero buffer/length");
+			register_device_error(dev, "Zero descriptor from device");
 			return -1;
 		}
 

windows/hid.c

@@ -1474,13 +1474,16 @@ int HID_API_EXPORT_CALL HID_API_CALL hid_get_serial_number_string(hid_device *de
 	return 0;
 }
 
-HID_API_EXPORT struct hid_device_info * HID_API_CALL hid_get_device_info(hid_device *dev) {
+HID_API_EXPORT struct hid_device_info * HID_API_CALL hid_get_device_info(hid_device *dev)
+{
 	if (!dev->device_info)
 	{
 		register_string_error(dev, L"NULL device info");
 		return NULL;
 	}
 
+	register_string_error(dev, NULL);
+
 	return dev->device_info;
 }
 
@@ -1565,10 +1568,18 @@ int HID_API_EXPORT_CALL hid_get_report_descriptor(hid_device *dev, unsigned char
 		return -1;
 	}
 
+
 	int res = hid_winapi_descriptor_reconstruct_pp_data(pp_data, buf, buf_size);
 
 	HidD_FreePreparsedData(pp_data);
 
+	if (res == 0) {
+		register_string_error(dev, NULL);
+	}
+	else {
+		register_string_error(dev, L"Failed to reconstruct descriptor from PREPARSED_DATA");
+	}
+
 	return res;
 }