Hi,
After trying some fuzzing on libavcodec, it seems that a lot of decoders
does not check (or not enough) for buffer overread which can lead for some
to a segfault.
I attached a patch that make get_bits.h function checked for overread by
default but let safe decoders disabling the checks at compilation time by
defining UNCHECK_BITSTREAM_READER before including get_bits.h.
If such patch would be including, I would gladly provide a patch
adding the #define UNCHECK_BITSTREAM_READER to the decoder that are 'safe'.
I haven't yet benchmark the performance loss but will do so.
One decoder breaks with this patch: mpegaudio. It seems to do weird things
with two get bit context and switching them while decoding. I will try to
have a look at it (unless someone would volunteer to explain me what it is
doing :)
Also, I haven't implemented the checks for A32_BITSTREAM_READER. But I am not
sure when (or even if) this reader is used.
Regards,
--
fenrir
>From 51abf83451b1f17283035c5b0742df1d01a09394 Mon Sep 17 00:00:00 2001
From: Laurent Aimar <fen...@videolan.org>
Date: Fri, 9 Sep 2011 00:52:36 +0200
Subject: [PATCH] Prevent by default for overread in get_bits.h functions.
It can be disabled (at compilation time so without any performance loss)
for decoder that check for overreads by themselves by defining
UNCHECK_BITSTREAM_READER before including get_bits.h
Checks for A32_BITSTREAM_READER are not yet implemented.
---
libavcodec/get_bits.h | 27 ++++++++++++++++++++++++++-
1 files changed, 26 insertions(+), 1 deletions(-)
diff --git a/libavcodec/get_bits.h b/libavcodec/get_bits.h
index d2ae345..e3fedb9 100644
--- a/libavcodec/get_bits.h
+++ b/libavcodec/get_bits.h
@@ -35,6 +35,9 @@
#include "libavutil/log.h"
#include "mathops.h"
+/* You can define UNCHECK_BITSTREAM_READER before including this file to
+ * avoid the penalty cost of checking for overread */
+
#if defined(ALT_BITSTREAM_READER_LE) && !defined(ALT_BITSTREAM_READER)
# define ALT_BITSTREAM_READER
#endif
@@ -127,6 +130,7 @@ for examples see get_bits, show_bits, skip_bits, get_vlc
# define OPEN_READER(name, gb) \
unsigned int name##_index = (gb)->index; \
+ unsigned int av_unused name##_size_in_bits = (gb)->size_in_bits; \
unsigned int av_unused name##_cache = 0
# define CLOSE_READER(name, gb) (gb)->index = name##_index
@@ -144,7 +148,15 @@ for examples see get_bits, show_bits, skip_bits, get_vlc
# endif
// FIXME name?
-# define SKIP_COUNTER(name, gb, num) name##_index += (num)
+#ifndef UNCHECK_BITSTREAM_READER
+# define SKIP_COUNTER(name, gb, num) do { \
+ name##_index += (num); \
+ if (name##_index > name##_size_in_bits) \
+ name##_index = name##_size_in_bits; \
+ } while (0)
+#else
+# define SKIP_COUNTER(name, gb, num) name##_index += (num);
+#endif
# define SKIP_BITS(name, gb, num) do { \
SKIP_CACHE(name, gb, num); \
@@ -172,10 +184,18 @@ static inline int get_bits_count(const GetBitContext *s){
static inline void skip_bits_long(GetBitContext *s, int n){
s->index += n;
+#ifndef UNCHECK_BITSTREAM_READER
+ if (s->index > s->size_in_bits)
+ s->index = s->size_in_bits;
+#endif
}
#elif defined A32_BITSTREAM_READER
+#ifndef UNCHECK_BITSTREAM_READER
+# warn "Checked bistream reader unimplemented"
+#endif
+
# define MIN_CACHE_BITS 32
# define OPEN_READER(name, gb) \
@@ -311,7 +331,12 @@ static inline unsigned int get_bits1(GetBitContext *s){
result <<= index & 7;
result >>= 8 - 1;
#endif
+#ifndef UNCHECK_BITSTREAM_READER
+ if (index < s->size_in_bits)
+ index++;
+#else
index++;
+#endif
s->index = index;
return result;
--
1.7.2.5
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
