| Message ID | 20190830214649.27761-1-benquike@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] Fix an OOB bug in parse_audio_mixer_unit | expand |
This is the backported patch of the following bug to v4.4.x and v4.14.x:
daac07156b33 ("ALSA: usb-audio: Fix an OOB bug in parse_audio_mixer_unit")
On Fri, Aug 30, 2019 at 5:47 PM Hui Peng <benquike@gmail.com> wrote:
> The `uac_mixer_unit_descriptor` shown as below is read from the
> device side. In `parse_audio_mixer_unit`, `baSourceID` field is
> accessed from index 0 to `bNrInPins` - 1, the current implementation
> assumes that descriptor is always valid (the length of descriptor
> is no shorter than 5 + `bNrInPins`). If a descriptor read from
> the device side is invalid, it may trigger out-of-bound memory
> access.
>
> ```
> struct uac_mixer_unit_descriptor {
> __u8 bLength;
> __u8 bDescriptorType;
> __u8 bDescriptorSubtype;
> __u8 bUnitID;
> __u8 bNrInPins;
> __u8 baSourceID[];
> }
> ```
>
> This patch fixes the bug by add a sanity check on the length of
> the descriptor.
>
> CVE: CVE-2018-15117
>
> Reported-by: Hui Peng <benquike@gmail.com>
> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
> Signed-off-by: Hui Peng <benquike@gmail.com>
> ---
> sound/usb/mixer.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index 1f7eb3816cd7..10ddec76f906 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -1628,6 +1628,7 @@ static int parse_audio_mixer_unit(struct mixer_build
> *state, int unitid,
> int pin, ich, err;
>
> if (desc->bLength < 11 || !(input_pins = desc->bNrInPins) ||
> + desc->bLength < sizeof(*desc) + desc->bNrInPins ||
> !(num_outs = uac_mixer_unit_bNrChannels(desc))) {
> usb_audio_err(state->chip,
> "invalid MIXER UNIT descriptor %d\n",
> --
> 2.17.1
>
>
On Fri, Aug 30, 2019 at 05:46:49PM -0400, Hui Peng wrote: > The `uac_mixer_unit_descriptor` shown as below is read from the > device side. In `parse_audio_mixer_unit`, `baSourceID` field is > accessed from index 0 to `bNrInPins` - 1, the current implementation > assumes that descriptor is always valid (the length of descriptor > is no shorter than 5 + `bNrInPins`). If a descriptor read from > the device side is invalid, it may trigger out-of-bound memory > access. > > ``` > struct uac_mixer_unit_descriptor { > __u8 bLength; > __u8 bDescriptorType; > __u8 bDescriptorSubtype; > __u8 bUnitID; > __u8 bNrInPins; > __u8 baSourceID[]; > } > ``` > > This patch fixes the bug by add a sanity check on the length of > the descriptor. > > CVE: CVE-2018-15117 FWIW, the correct CVE id should be probably CVE-2019-15117 here. But there was already a patch queued and released in 5.2.10 and 4.19.68 for this issue (as far I can see; is this correct?) Regards, Salvatore
On 9/1/19 8:58 AM, Salvatore Bonaccorso wrote: > On Fri, Aug 30, 2019 at 05:46:49PM -0400, Hui Peng wrote: >> The `uac_mixer_unit_descriptor` shown as below is read from the >> device side. In `parse_audio_mixer_unit`, `baSourceID` field is >> accessed from index 0 to `bNrInPins` - 1, the current implementation >> assumes that descriptor is always valid (the length of descriptor >> is no shorter than 5 + `bNrInPins`). If a descriptor read from >> the device side is invalid, it may trigger out-of-bound memory >> access. >> >> ``` >> struct uac_mixer_unit_descriptor { >> __u8 bLength; >> __u8 bDescriptorType; >> __u8 bDescriptorSubtype; >> __u8 bUnitID; >> __u8 bNrInPins; >> __u8 baSourceID[]; >> } >> ``` >> >> This patch fixes the bug by add a sanity check on the length of >> the descriptor. >> >> CVE: CVE-2018-15117 > FWIW, the correct CVE id should be probably CVE-2019-15117 here. Yes, the CVE id was wrong. I have updated it in the attached patch. > But there was already a patch queued and released in 5.2.10 and > 4.19.68 for this issue (as far I can see; is this correct?) Yes, it should have been fixed in those branches. But google asked me to back port it to v4.4.190 and v4.14.141. I have mentioned it in one previous email, but it was blocked by vger because it was sent in html format. Can you apply it to these 2 versions? (it applies to both versions) Thanks. > Regards, > Salvatore
On Fri, Aug 30, 2019 at 05:49:59PM -0400, Hui Peng wrote: > This is the backported patch of the following bug to v4.4.x and v4.14.x: > daac07156b33 ("ALSA: usb-audio: Fix an OOB bug in parse_audio_mixer_unit") Thanks, also now queued up to 4.9.y, you forgot that one :) greg k-h
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index 1f7eb3816cd7..10ddec76f906 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -1628,6 +1628,7 @@ static int parse_audio_mixer_unit(struct mixer_build *state, int unitid, int pin, ich, err; if (desc->bLength < 11 || !(input_pins = desc->bNrInPins) || + desc->bLength < sizeof(*desc) + desc->bNrInPins || !(num_outs = uac_mixer_unit_bNrChannels(desc))) { usb_audio_err(state->chip, "invalid MIXER UNIT descriptor %d\n",
The `uac_mixer_unit_descriptor` shown as below is read from the device side. In `parse_audio_mixer_unit`, `baSourceID` field is accessed from index 0 to `bNrInPins` - 1, the current implementation assumes that descriptor is always valid (the length of descriptor is no shorter than 5 + `bNrInPins`). If a descriptor read from the device side is invalid, it may trigger out-of-bound memory access. ``` struct uac_mixer_unit_descriptor { __u8 bLength; __u8 bDescriptorType; __u8 bDescriptorSubtype; __u8 bUnitID; __u8 bNrInPins; __u8 baSourceID[]; } ``` This patch fixes the bug by add a sanity check on the length of the descriptor. CVE: CVE-2018-15117 Reported-by: Hui Peng <benquike@gmail.com> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net> Signed-off-by: Hui Peng <benquike@gmail.com> --- sound/usb/mixer.c | 1 + 1 file changed, 1 insertion(+)