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

[Takashi Iwai]

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