[alsa-devel] [PATCH] Fix an OOB bug in uac_mixer_unit_bmControls

[Hui Peng]

Sorry, this is the wrong patch. I will send it again.

On Mon, Aug 19, 2019 at 6:00 PM Hui Peng <benquike at gmail.com> wrote:

> `uac_mixer_unit_get_channels` calls `uac_mixer_unit_bmControls`
> to get pointer to bmControls field. The current implementation of
> `uac_mixer_unit_get_channels` does properly check the size of
> uac_mixer_unit_descriptor descriptor and may allow OOB access
> in `uac_mixer_unit_bmControls`.
>
> Reported-by: Hui Peng <benquike at gmail.com>
> Reported-by: Mathias Payer <mathias.payer at nebelwelt.net>
> Signed-off-by: Hui Peng <benquike at gmail.com>
> ---
>  sound/usb/mixer.c | 25 ++++++++++++++++++-------
>  1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index b5927c3d5bc0..00e6274a63c3 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -738,28 +738,39 @@ static int get_cluster_channels_v3(struct
> mixer_build *state, unsigned int clust
>  static int uac_mixer_unit_get_channels(struct mixer_build *state,
>                                        struct uac_mixer_unit_descriptor
> *desc)
>  {
> -       int mu_channels;
> +       int mu_channels = 0;
>         void *c;
>
> -       if (desc->bLength < sizeof(*desc))
> -               return -EINVAL;
>         if (!desc->bNrInPins)
>                 return -EINVAL;
> -       if (desc->bLength < sizeof(*desc) + desc->bNrInPins)
> -               return -EINVAL;
>
>         switch (state->mixer->protocol) {
>         case UAC_VERSION_1:
> +               // limit derived from uac_mixer_unit_bmControls
> +               if (desc->bLength < sizeof(*desc) + desc->bNrInPins + 4)
> +                       return 0;
> +
> +               mu_channels = uac_mixer_unit_bNrChannels(desc);
> +               break;
> +
>         case UAC_VERSION_2:
> -       default:
> -               if (desc->bLength < sizeof(*desc) + desc->bNrInPins + 1)
> +               // limit derived from uac_mixer_unit_bmControls
> +               if (desc->bLength < sizeof(*desc) + desc->bNrInPins + 6)
>                         return 0; /* no bmControls -> skip */
> +
>                 mu_channels = uac_mixer_unit_bNrChannels(desc);
>                 break;
>         case UAC_VERSION_3:
> +               // limit derived from uac_mixer_unit_bmControls
> +               if (desc->bLength < sizeof(*desc) + desc->bNrInPins + 2)
> +                       return 0; /* no bmControls -> skip */
> +
>                 mu_channels = get_cluster_channels_v3(state,
>                                 uac3_mixer_unit_wClusterDescrID(desc));
>                 break;
> +
> +       default:
> +               break;
>         }
>
>         if (!mu_channels)
> --
> 2.22.1
>
>