[FFmpeg-devel] [PATCH] dvbsubdec: check against buffer overreads

[Janne Grunau]

On Wed, Feb 09, 2011 at 11:23:54PM +0000, M?ns Rullg?rd wrote:
> Janne Grunau <janne-ffmpeg at jannau.net> writes:
> 
> > @@ -1437,6 +1439,11 @@ static int dvbsub_decode(AVCodecContext *avctx,
> >          segment_length = AV_RB16(p);
> >          p += 2;
> >
> > +        if (p+segment_length > p_end) {
> > +            av_dlog(avctx, "incomplete packet");
> > +            return -1;
> > +        }
> 
> Not overflow-safe.  If the buffer is near the top of the address space,
> adding an oversize segment_length could make it wrap.  More generally,
> the C standard makes so much as calculating an invalid address undefined
> behaviour.
> 
> The correct test for error here is (p_end - p < segment_length).

thanks, fixed

Janne
---8<---
Signed-off-by: Janne Grunau <janne-ffmpeg at jannau.net>
---
 libavcodec/dvbsubdec.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c
index 401144f..dbeb24a 100644
--- a/libavcodec/dvbsubdec.c
+++ b/libavcodec/dvbsubdec.c
@@ -1423,13 +1423,15 @@ static int dvbsub_decode(AVCodecContext *avctx,
 
 #endif
 
-    if (buf_size <= 2 || *buf != 0x0f)
+    if (buf_size <= 6 || *buf != 0x0f) {
+        av_dlog(avctx, "incomplete or broken packet");
         return -1;
+    }
 
     p = buf;
     p_end = buf + buf_size;
 
-    while (p < p_end && *p == 0x0f) {
+    while (p+6 < p_end && *p == 0x0f) {
         p += 1;
         segment_type = *p++;
         page_id = AV_RB16(p);
@@ -1437,6 +1439,11 @@ static int dvbsub_decode(AVCodecContext *avctx,
         segment_length = AV_RB16(p);
         p += 2;
 
+        if (p_end - p < segment_length) {
+            av_dlog(avctx, "incomplete or broken packet");
+            return -1;
+        }
+
         if (page_id == ctx->composition_id || page_id == ctx->ancillary_id ||
             ctx->composition_id == -1 || ctx->ancillary_id == -1) {
             switch (segment_type) {
-- 
1.7.4