Bugzilla – Bug 1054127
VUL-0: CVE-2016-5863: kernel-source: usbhid: ioctl handler in the Linux kernel, several sanity checks are missing which can lead to out-of-bounds accesses.
Last modified: 2017-08-21 14:07:15 UTC
CVE-2016-5863 In an ioctl handler in all Qualcomm products with Android releases from CAF using the Linux kernel, several sanity checks are missing which can lead to out-of-bounds accesses. References: http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2016-5863 http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-5863 https://source.codeaurora.org/quic/la/kernel/msm-3.18/commit/?id=daf0acd54a6a80de227baef9a06285e4aa5f8c93
This was already fixed in mainline by commit 93a2001bdfd5376c3dc2158653034c20392d15c5 Author: Scott Bauer <sbauer@plzdonthack.me> Date: Thu Jun 23 08:59:47 2016 -0600 HID: hiddev: validate num_values for HIDIOCGUSAGES, HIDIOCSUSAGES commands This patch validates the num_values parameter from userland during the HIDIOCGUSAGES and HIDIOCSUSAGES commands. Previously, if the report id was set to HID_REPORT_ID_UNKNOWN, we would fail to validate the num_values parameter leading to a heap overflow. Cc: stable@vger.kernel.org Signed-off-by: Scott Bauer <sbauer@plzdonthack.me> Signed-off-by: Jiri Kosina <jkosina@suse.cz> and so is a duplicated of bug 986572 aka CVE-2016-5829 *** This bug has been marked as a duplicate of bug 986572 ***
that said, the patch is a bit different. It also moves out this block out of the else branch, which the mainline fix does not do. if (cmd == HIDIOCGCOLLECTIONINDEX) { if (uref->usage_index >= field->maxusage) goto inval; } else if (uref->usage_index >= field->report_count) goto inval; So half is fixed, rest needs review.
Oliver, can you please check whether we are still vulnerable or not?
(In reply to Marcus Meissner from comment #2) > that said, the patch is a bit different. > > It also moves out this block out of the else branch, which the mainline fix > does not do. > > if (cmd == HIDIOCGCOLLECTIONINDEX) { > if (uref->usage_index >= field->maxusage) > goto inval; > } else if (uref->usage_index >= field->report_count) > goto inval; > > So half is fixed, rest needs review. If this code branch without the sanity check is executed if (uref->report_id == HID_REPORT_ID_UNKNOWN) hiddev_lookup_usage() is executed. If rewrites some fields of uref if (field->usage[j].hid == uref->usage_code) { uref->report_id = report->id; uref->field_index = i; uref->usage_index = j; Thus uref->usage_index in that case comes from a kernel data structure not provided by user land, hence need not be validated. This code and the upstream fix leaving the check conditional is secure; readable code it is not.
ok, thanks for checking!
dup of bug 986572 *** This bug has been marked as a duplicate of bug 986572 ***