[vlc-devel] [PATCH] packetizer/h264: fix 17585: prevent overflow leading to crash

[Filip Roséen]

Given that h264_sequence_parameter_set_t::i_id and
h264_picture_parameter_set_t::{i_id,i_sps_id} are read directly from
the stream as uint32_t, it does not make much sense storing them as
ints.

The sample attached to ticket 17585 crashes due to an overflow issue
when assigning the read value to the previous data-member type,
causing a negative value to be stored (circumventing all checks to see
whether the value is bigger than H264_MAX_{PPS,SPS}).

By changing the data-members types from int to uint32_t we make sure
that no overflow error can occur during the read, and that the value
is always in the range [0, UINT32_MAX] - making checks such as
"p_pps->i_id <= H264_MAX_PPS" valid in terms of validation.

fixes #17585
---
 modules/packetizer/h264.c     | 8 ++++++--
 modules/packetizer/h264_nal.h | 6 +++---
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/modules/packetizer/h264.c b/modules/packetizer/h264.c
index fc72726..58a6ca6 100644
--- a/modules/packetizer/h264.c
+++ b/modules/packetizer/h264.c
@@ -767,7 +767,7 @@ static void PutSPS( decoder_t *p_dec, block_t *p_frag )
     }
     /* We have a new SPS */
     if( !p_sys->b_sps )
-        msg_Dbg( p_dec, "found NAL_SPS (sps_id=%d)", p_sps->i_id );
+        msg_Dbg( p_dec, "found NAL_SPS (sps_id=%" PRIu32 ")", p_sps->i_id );
     p_sys->b_sps = true;
 
     if( p_sys->pp_sps[p_sps->i_id] )
@@ -797,7 +797,11 @@ static void PutPPS( decoder_t *p_dec, block_t *p_frag )
 
     /* We have a new PPS */
     if( !p_sys->b_pps )
-        msg_Dbg( p_dec, "found NAL_PPS (pps_id=%d sps_id=%d)", p_pps->i_id, p_pps->i_sps_id );
+    {
+        msg_Dbg( p_dec, "found NAL_PPS (pps_id=%" PRIu32 " sps_id=%" PRIu32 ")",
+            p_pps->i_id, p_pps->i_sps_id );
+    }
+
     p_sys->b_pps = true;
 
     if( p_sys->pp_pps[p_pps->i_id] )
diff --git a/modules/packetizer/h264_nal.h b/modules/packetizer/h264_nal.h
index 201f1ae..8bc3205 100644
--- a/modules/packetizer/h264_nal.h
+++ b/modules/packetizer/h264_nal.h
@@ -84,7 +84,7 @@ void h264_release_pps( h264_picture_parameter_set_t * );
 
 struct h264_sequence_parameter_set_t
 {
-    int i_id;
+    uint32_t i_id;
     uint8_t i_profile, i_level;
     uint8_t i_constraint_set_flags;
     /* according to avcC, 3 bits max for those */
@@ -129,8 +129,8 @@ struct h264_sequence_parameter_set_t
 
 struct h264_picture_parameter_set_t
 {
-    int i_id;
-    int i_sps_id;
+    uint32_t i_id;
+    uint32_t i_sps_id;
     int i_pic_order_present_flag;
 };
 
-- 
2.10.1