diff mbox series

[FFmpeg-devel,v2,1/3] avformat/rcwtenc: Fix potential out-of-bounds write

Message ID AS8P250MB0744E80FACE3A6B5751F670E8F442@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM
State Accepted
Commit d30fe36b88983feb7c48fc22a4bc03868248c479
Headers show
Series [FFmpeg-devel,v2,1/3] avformat/rcwtenc: Fix potential out-of-bounds write | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Andreas Rheinhardt Feb. 8, 2024, 3:17 p.m. UTC 
The rcwt muxer uses several counters for how much data
it has already cached: One byte counter and one counter
for how many complete blocks (of three bytes each).
These counters can become inconsistent when the muxer is
fed incomplete blocks as the muxer presumes that it is
about to write a new block at the start of each write_packet
call. E.g. sending 65535*3+1 1-byte packets (with data[0] e.g. 0x03)
will trigger an out-of-bounds write.

This patch fixes this by processing the data in complete blocks
only. This also allows to simplify the code, e.g. to remove one of
the counters.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
Now using RCWT_BLOCK_SIZE where it makes sense.
The other two patches are unchanged and therefore not sent.

 libavformat/rcwtenc.c | 38 +++++++++++---------------------------
 1 file changed, 11 insertions(+), 27 deletions(-)
diff mbox series

Patch

diff --git a/libavformat/rcwtenc.c b/libavformat/rcwtenc.c
index 839436ce84..1becef5f48 100644
--- a/libavformat/rcwtenc.c
+++ b/libavformat/rcwtenc.c
@@ -65,7 +65,6 @@ 
 #define RCWT_BLOCK_SIZE                     3
 
 typedef struct RCWTContext {
-    int cluster_nb_blocks;
     int cluster_pos;
     int64_t cluster_pts;
     uint8_t cluster_buf[RCWT_CLUSTER_MAX_BLOCKS * RCWT_BLOCK_SIZE];
@@ -75,7 +74,6 @@  static void rcwt_init_cluster(AVFormatContext *avf)
 {
     RCWTContext *rcwt = avf->priv_data;
 
-    rcwt->cluster_nb_blocks = 0;
     rcwt->cluster_pos = 0;
     rcwt->cluster_pts = AV_NOPTS_VALUE;
     memset(rcwt->cluster_buf, 0, sizeof(rcwt->cluster_buf));
@@ -85,10 +83,10 @@  static void rcwt_flush_cluster(AVFormatContext *avf)
 {
     RCWTContext *rcwt = avf->priv_data;
 
-    if (rcwt->cluster_nb_blocks > 0) {
+    if (rcwt->cluster_pos > 0) {
         avio_wl64(avf->pb, rcwt->cluster_pts);
-        avio_wl16(avf->pb, rcwt->cluster_nb_blocks);
-        avio_write(avf->pb, rcwt->cluster_buf, (rcwt->cluster_nb_blocks * RCWT_BLOCK_SIZE));
+        avio_wl16(avf->pb, rcwt->cluster_pos / RCWT_BLOCK_SIZE);
+        avio_write(avf->pb, rcwt->cluster_buf, rcwt->cluster_pos);
     }
 
     rcwt_init_cluster(avf);
@@ -129,10 +127,7 @@  static int rcwt_write_packet(AVFormatContext *avf, AVPacket *pkt)
 {
     RCWTContext *rcwt = avf->priv_data;
 
-    int in_block = 0;
-    int nb_block_bytes = 0;
-
-    if (pkt->size == 0)
+    if (pkt->size < RCWT_BLOCK_SIZE)
         return 0;
 
     /* new PTS, new cluster */
@@ -146,11 +141,11 @@  static int rcwt_write_packet(AVFormatContext *avf, AVPacket *pkt)
         return 0;
     }
 
-    for (int i = 0; i < pkt->size; i++) {
+    for (int i = 0; i <= pkt->size - RCWT_BLOCK_SIZE;) {
         uint8_t cc_valid;
         uint8_t cc_type;
 
-        if (rcwt->cluster_nb_blocks == RCWT_CLUSTER_MAX_BLOCKS) {
+        if (rcwt->cluster_pos == RCWT_CLUSTER_MAX_BLOCKS * RCWT_BLOCK_SIZE) {
             av_log(avf, AV_LOG_WARNING, "Starting new cluster due to size\n");
             rcwt_flush_cluster(avf);
         }
@@ -158,25 +153,14 @@  static int rcwt_write_packet(AVFormatContext *avf, AVPacket *pkt)
         cc_valid = (pkt->data[i] & 0x04) >> 2;
         cc_type = pkt->data[i] & 0x03;
 
-        if (!in_block && !(cc_valid || cc_type == 3))
-            continue;
-
-        memcpy(&rcwt->cluster_buf[rcwt->cluster_pos], &pkt->data[i], 1);
-        rcwt->cluster_pos++;
-
-        if (!in_block) {
-            in_block = 1;
-            nb_block_bytes = 1;
+        if (!(cc_valid || cc_type == 3)) {
+            i++;
             continue;
         }
 
-        nb_block_bytes++;
-
-        if (nb_block_bytes == RCWT_BLOCK_SIZE) {
-            in_block = 0;
-            nb_block_bytes = 0;
-            rcwt->cluster_nb_blocks++;
-        }
+        memcpy(&rcwt->cluster_buf[rcwt->cluster_pos], &pkt->data[i], 3);
+        rcwt->cluster_pos += 3;
+        i                 += 3;
     }
 
     return 0;