[alsa-devel] Suspected heap overflow in snd_midi_event_encode_byte()

[Prashant Malani]

Thanks.

Curious to know: Whats the URL for alsa-lib git tree? I'd love to be able
to check it out.

On Wed, Aug 22, 2018 at 11:46 PM, Takashi Iwai <tiwai at suse.de> wrote:

> On Wed, 22 Aug 2018 23:32:35 +0200,
> Prashant Malani wrote:
> >
> > Thanks Takashi,
> >
> > I can confirm that this patch fixes the heap overflow issue.
> > Could you kindly submit this patch into the alsa-lib tree?
>
> Now applied the following patch to git tree.
>
>
> thanks,
>
> Takashi
>
> -- 8< --
> From: Takashi Iwai <tiwai at suse.de>
> Subject: [PATCH] seq: Fix signedness in MIDI encoder/decoder
>
> The qlen field of struct snd_midi_event was declared as size_t while
> status_events[] assigns the qlen to -1 indicating to skip.  This leads
> to the misinterpretation since size_t is unsigned, hence it passes the
> check "dev.qlen > 0" incorrectly in snd_midi_event_encode_byte(),
> which eventually results in a memory corruption.
>
> Also, snd_midi_event_decode() doesn't consider about a negative qlen
> value and tries to copy the size as is.
>
> This patch fixes these issues: the first one is addressed by simply
> replacing size_t with ssize_t in snd_midi_event struct.  For the
> latter, a check "qlen <= 0" is added to bail out; this is also good as
> a slight optimization.
>
> Reported-by: Prashant Malani <pmalani at chromium.org>
> Signed-off-by: Takashi Iwai <tiwai at suse.de>
> ---
>  src/seq/seq_midi_event.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/seq/seq_midi_event.c b/src/seq/seq_midi_event.c
> index 2e7d1035442a..5a12a18ce781 100644
> --- a/src/seq/seq_midi_event.c
> +++ b/src/seq/seq_midi_event.c
> @@ -35,7 +35,7 @@
>
>  /* midi status */
>  struct snd_midi_event {
> -       size_t qlen;    /* queue length */
> +       ssize_t qlen;   /* queue length */
>         size_t read;    /* chars read */
>         int type;       /* current event type */
>         unsigned char lastcmd;
> @@ -606,6 +606,8 @@ long snd_midi_event_decode(snd_midi_event_t *dev,
> unsigned char *buf, long count
>                                 status_event[type].decode(ev, xbuf + 0);
>                         qlen = status_event[type].qlen;
>                 }
> +               if (qlen <= 0)
> +                       return 0;
>                 if (count < qlen)
>                         return -ENOMEM;
>                 memcpy(buf, xbuf, qlen);
> --
> 2.18.0
>
>