h264: Handle over-sized frames

Over-sized frames from the server need to be handled to avoid out-of-bounds memory access.
TigerVNC · Jul 30, 2024 · 9e36dba · 9e36dba
1 parent e2a8cd0
commit 9e36dba
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 9 deletions.
diff --git a/common/rfb/H264LibavDecoderContext.cxx b/common/rfb/H264LibavDecoderContext.cxx
@@ -21,6 +21,7 @@
 
 extern "C" {
 #include <libavutil/imgutils.h>
+#include <libavutil/cpu.h>
 #include <libavcodec/version.h>
 }
 #if LIBAVCODEC_VERSION_MAJOR > 57 || LIBAVCODEC_VERSION_MAJOR == 57 && LIBAVCODEC_VERSION_MINOR >= 37
@@ -43,7 +44,6 @@ bool H264LibavDecoderContext::initCodec() {
   os::AutoMutex lock(&mutex);
 
   sws = nullptr;
-  swsBuffer = nullptr;
   h264WorkBuffer = nullptr;
   h264WorkBufferLength = 0;
 
@@ -87,9 +87,6 @@ bool H264LibavDecoderContext::initCodec() {
     return false;
   }
 
-  int numBytes = av_image_get_buffer_size(AV_PIX_FMT_RGB32, rect.width(), rect.height(), 1);
-  swsBuffer = new uint8_t[numBytes];
-
   initialized = true;
   return true;
 }
@@ -102,7 +99,6 @@ void H264LibavDecoderContext::freeCodec() {
   av_parser_close(parser);
   avcodec_free_context(&avctx);
   av_frame_free(&frame);
-  delete[] swsBuffer;
   free(h264WorkBuffer);
   initialized = false;
 }
@@ -222,9 +218,16 @@ void H264LibavDecoderContext::decode(const uint8_t* h264_in_buffer,
 
   int stride;
   pb->getBuffer(rect, &stride);
-  int dst_linesize = rect.width() * pb->getPF().bpp / 8;
 
-  sws_scale(sws, frame->data, frame->linesize, 0, frame->height, &swsBuffer, &dst_linesize);
+  int dstBytesPerPixel = pb->getPF().bpp / 8;
+  // The destination buffer must be aligned for SIMD operations in sws_scale().
+  int dstLineSize = FFALIGN(frame->width * dstBytesPerPixel, av_cpu_max_align());
+
+  swsBuffer.resize(dstLineSize * frame->height, 0);
+  auto swsBufferData = swsBuffer.data();
+
+  sws_scale(sws, frame->data, frame->linesize, 0, frame->height, &swsBufferData,
+            &dstLineSize);
 
-  pb->imageRect(rect, swsBuffer);
+  pb->imageRect(rect, swsBufferData, dstLineSize / dstBytesPerPixel);
 }
diff --git a/common/rfb/H264LibavDecoderContext.h b/common/rfb/H264LibavDecoderContext.h
@@ -28,6 +28,8 @@ extern "C" {
 
 #include <rfb/H264DecoderContext.h>
 
+#include <vector>
+
 namespace rfb {
   class H264LibavDecoderContext : public H264DecoderContext {
     public:
@@ -48,7 +50,7 @@ namespace rfb {
       AVCodecParserContext *parser;
       AVFrame* frame;
       SwsContext* sws;
-      uint8_t* swsBuffer;
+      std::vector<uint8_t> swsBuffer;
       uint8_t* h264WorkBuffer;
       uint32_t h264WorkBufferLength;
   };