[FFmpeg-devel] [PATCH] libvorbis.c: use memmove() when source/dest overlap and add buffer checks

[Thierry Foucu]

Hi,

$subject

Find these problems when using version 1.3.1 of the vorbis library


Index: libavcodec/libvorbis.c
===================================================================
--- libavcodec/libvorbis.c (revision 23079)
+++ libavcodec/libvorbis.c (working copy)
@@ -171,9 +171,13 @@
              * not, apparently the end of stream decision is in libogg. */
             if(op.bytes==1)
                 continue;
+            if (context->buffer_index + sizeof(ogg_packet) + op.bytes >
BUFFER_SIZE) {
+              av_log(avccontext, AV_LOG_ERROR, "libvorbis: buffer
overflow.");
+              return -1;
+            }
             memcpy(context->buffer + context->buffer_index, &op,
sizeof(ogg_packet));
             context->buffer_index += sizeof(ogg_packet);
-            memcpy(context->buffer + context->buffer_index, op.packet,
op.bytes);
+            memmove(context->buffer + context->buffer_index, op.packet,
op.bytes);
             context->buffer_index += op.bytes;
 //            av_log(avccontext, AV_LOG_DEBUG, "e%d / %d\n",
context->buffer_index, op.bytes);
         }
@@ -188,9 +192,13 @@
         avccontext->coded_frame->pts= av_rescale_q(op2->granulepos,
(AVRational){1, avccontext->sample_rate}, avccontext->time_base);
         //FIXME we should reorder the user supplied pts and not assume that
they are spaced by 1/sample_rate

+        if (l > buf_size) {
+          av_log(avccontext, AV_LOG_ERROR, "libvorbis: buffer overflow.");
+          return -1;
+        }
         memcpy(packets, op2->packet, l);
         context->buffer_index -= l + sizeof(ogg_packet);
-        memcpy(context->buffer, context->buffer + l + sizeof(ogg_packet),
context->buffer_index);
+        memmove(context->buffer, context->buffer + l + sizeof(ogg_packet),
context->buffer_index);
 //        av_log(avccontext, AV_LOG_DEBUG, "E%d\n", l);
     }
-------------- next part --------------
Index: libavcodec/libvorbis.c
===================================================================
--- libavcodec/libvorbis.c	(revision 23079)
+++ libavcodec/libvorbis.c	(working copy)
@@ -171,9 +171,13 @@
              * not, apparently the end of stream decision is in libogg. */
             if(op.bytes==1)
                 continue;
+            if (context->buffer_index + sizeof(ogg_packet) + op.bytes > BUFFER_SIZE) {
+              av_log(avccontext, AV_LOG_ERROR, "libvorbis: buffer overflow.");
+              return -1;
+            }
             memcpy(context->buffer + context->buffer_index, &op, sizeof(ogg_packet));
             context->buffer_index += sizeof(ogg_packet);
-            memcpy(context->buffer + context->buffer_index, op.packet, op.bytes);
+            memmove(context->buffer + context->buffer_index, op.packet, op.bytes);
             context->buffer_index += op.bytes;
 //            av_log(avccontext, AV_LOG_DEBUG, "e%d / %d\n", context->buffer_index, op.bytes);
         }
@@ -188,9 +192,13 @@
         avccontext->coded_frame->pts= av_rescale_q(op2->granulepos, (AVRational){1, avccontext->sample_rate}, avccontext->time_base);
         //FIXME we should reorder the user supplied pts and not assume that they are spaced by 1/sample_rate
 
+        if (l > buf_size) {
+          av_log(avccontext, AV_LOG_ERROR, "libvorbis: buffer overflow.");
+          return -1;
+        }
         memcpy(packets, op2->packet, l);
         context->buffer_index -= l + sizeof(ogg_packet);
-        memcpy(context->buffer, context->buffer + l + sizeof(ogg_packet), context->buffer_index);
+        memmove(context->buffer, context->buffer + l + sizeof(ogg_packet), context->buffer_index);
 //        av_log(avccontext, AV_LOG_DEBUG, "E%d\n", l);
     }