Control: tags -1 + patch security
Hi again Henri,
Am Mittwoch, den 18.02.2015, 20:59 +0200 schrieb Henri Salo:
> I found another segmentation fault crash while fuzzing with AFL
> <http://lcamtuf.coredump.cx/afl/>. For some reason I can't get full backtrace
> with gdb.
now this is really only caused by the fact that num_channels has a
negative value. It was a bit tricky to investigate since the stack was
smashed (thus no backtrace) but the analysis should be reasonable.
The sample at hand reports to have num_channels = -251, and it is really
unbelievable that there is no early sanity check yet for this value.
However, in get_audio_common() the num_channels variable is set to this
value (l. 733), which is then multiplied with the value of
samples_to_read (= 576) and passed over to read_samples_pcm() (l. 800).
This function, in turn, passes the value of samples_to_read (now
-144576) over to unpack_read_samples() (l. 1289) together with a pointer
to sample_buffer which is a static int array of size 2304. In
unpack_read_samples() finally the value of samples_to_read is passed
over to a fread() call as the number of elements of size
"bytes_per_sample" (= 1) to read from the pcm_in stream (l. 1188).
The arguments in question of fread() are of type size_t, i.e. unsigned.
The value of samples_to_read (= -144576) translates to
18446744073709407040 as size_t type, i.e. "unlimited". And indeed
fread() returns 3967 bytes into the samples_read variable.
Unfortunately, these 3967 bytes have been written into the static int
array "sample_buffer" which was of size 2304. Boom, stack corrupted!
I suggest to fix this issue at its root and extend Maks' patch to also
bail out if (num_channels < 0). Patching the sample you provided to
num_channels = 1, LAME processes this file without problems. The
attached patch does that, simply copy it over the previous patch. Also,
I have set the "security" tag for this bug, because I think being able
to override chosen parts of the stack with data of your own choice is
rather critical.
- Fabian
From 1ea4eac3e7d57dbad42fb067a32ac1600a0397a0 Mon Sep 17 00:00:00 2001
From: Maks Naumov <maksq...@ukr.net>
Date: Thu, 22 Jan 2015 16:20:40 +0200
Subject: [PATCH] Add check for invalid input sample rate
Signed-off-by: Maks Naumov <maksq...@ukr.net>
---
libmp3lame/lame.c | 6 ++++++
1 file changed, 6 insertions(+)
--- a/libmp3lame/lame.c
+++ b/libmp3lame/lame.c
@@ -822,6 +822,12 @@ lame_init_params(lame_global_flags * gfp
}
#endif
+ if (gfp->samplerate_in < 0 || gfp->num_channels < 0) {
+ freegfc(gfc);
+ gfp->internal_flags = NULL;
+ return -1;
+ }
+
cfg->disable_reservoir = gfp->disable_reservoir;
cfg->lowpassfreq = gfp->lowpassfreq;
cfg->highpassfreq = gfp->highpassfreq;
_______________________________________________
pkg-multimedia-maintainers mailing list
pkg-multimedia-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-multimedia-maintainers
