CVE-2021-30020: Out-of-bounds Write (CWE-787)
value: In the function gf_hevc_read_pps_bs_internal function in media_tools/av_parsers.c in GPAC 1.0.1 there is a loop, which with crafted file, pps->num_tile_columns may be larger than sizeof(pps->column_width), which results in a heap overflow in the loop.
CWE description: The product writes data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 5.5
Published: 2021-04-19T20:15Z
Repository: https://github.com/gpac/gpac
Language: C
Files changed: src/media_tools/av_parsers.c
Commit date: 2021-03-29 09:38:29 +0200
Commit message: add safety in avc/hevc/vvc sps/pps/vps ID check - cf #1720 #1721 #1722

Diff: diff --git a/src/media_tools/av_parsers.c b/src/media_tools/av_parsers.c
index a30cfae9f73..2698bbca99c 100644
--- a/src/media_tools/av_parsers.c
+++ b/src/media_tools/av_parsers.c
@@ -5113,14 +5113,11 @@ static s32 gf_avc_read_sps_bs_internal(GF_BitStream *bs, AVCState *avc, u32 subs
 
 	/*SubsetSps is used to be sure that AVC SPS are not going to be scratched
 	by subset SPS. According to the SVC standard, subset SPS can have the same sps_id
 	than its base layer, but it does not refer to the same SPS. */
 	sps_id = gf_bs_read_ue_log(bs, "sps_id") + GF_SVC_SSPS_ID_SHIFT * subseq_sps;
-	if (sps_id >= 32) {
-		return -1;
-	}
-	if (sps_id < 0) {
+	if ((sps_id < 0) || (sps_id >= 32)) {
 		return -1;
 	}
 
 	luma_bd = chroma_bd = 0;
 	sps = &avc->sps[sps_id];
@@ -5445,19 +5442,19 @@ static s32 gf_avc_read_pps_bs_internal(GF_BitStream *bs, AVCState *avc, u32 nal_
 		gf_bs_read_int_log(bs, 1, "forbidden_zero_bit");
 		gf_bs_read_int_log(bs, 2, "nal_ref_idc");
 		gf_bs_read_int_log(bs, 5, "nal_unit_type");
 	}
 	pps_id = gf_bs_read_ue_log(bs, "pps_id");
-	if (pps_id >= 255) {
+	if ((pps_id<0) || (pps_id >= 255)) {
 		return -1;
 	}
 	pps = &avc->pps[pps_id];
 	pps->id = pps_id;
 
 	if (!pps->status) pps->status = 1;
 	pps->sps_id = gf_bs_read_ue_log(bs, "sps_id");
-	if (pps->sps_id >= 32) {
+	if ((pps->sps_id<0) || (pps->sps_id >= 32)) {
 		pps->sps_id = 0;
 		return -1;
 	}
 	/*sps_id may be refer to regular SPS or subseq sps, depending on the coded slice referring to the pps*/
 	if (!avc->sps[pps->sps_id].state && !avc->sps[pps->sps_id + GF_SVC_SSPS_ID_SHIFT].state) {
@@ -6841,11 +6838,11 @@ s32 hevc_parse_slice_segment(GF_BitStream *bs, HEVCState *hevc, HEVCSliceInfo *s
 	if (RapPicFlag) {
 		gf_bs_read_int_log(bs, 1, "no_output_of_prior_pics_flag");
 	}
 
 	pps_id = gf_bs_read_ue_log(bs, "pps_id");
-	if (pps_id >= 64)
+	if ((pps_id<0) || (pps_id >= 64))
 		return -1;
 
 	pps = &hevc->pps[pps_id];
 	sps = &hevc->sps[pps->sps_id];
 	si->sps = sps;
@@ -7664,11 +7661,11 @@ static s32 gf_hevc_read_vps_bs_internal(GF_BitStream *bs, HEVCState *hevc, Bool
 	u8 layer_id_included_flag[MAX_LHVC_LAYERS][64];
 
 	//nalu header already parsed
 	vps_id = gf_bs_read_int_log(bs, 4, "vps_id");
 
-	if (vps_id >= 16) return -1;
+	if ((vps_id<0) || (vps_id >= 16)) return -1;
 
 	vps = &hevc->vps[vps_id];
 	vps->bit_pos_vps_extensions = -1;
 	if (!vps->state) {
 		vps->id = vps_id;
@@ -7893,11 +7890,11 @@ static s32 gf_hevc_read_sps_bs_internal(GF_BitStream *bs, HEVCState *hevc, u8 la
 
 	if (vui_flag_pos) *vui_flag_pos = 0;
 
 	//nalu header already parsed
 	vps_id = gf_bs_read_int_log(bs, 4, "vps_id");
-	if (vps_id >= 16) {
+	if ((vps_id<0) || (vps_id >= 16)) {
 		return -1;
 	}
 	memset(&ptl, 0, sizeof(ptl));
 	max_sub_layers_minus1 = 0;
 	sps_ext_or_max_sub_layers_minus1 = 0;
@@ -8208,11 +8205,11 @@ static s32 gf_hevc_read_pps_bs_internal(GF_BitStream *bs, HEVCState *hevc)
 	if (!pps->state) {
 		pps->id = pps_id;
 		pps->state = 1;
 	}
 	pps->sps_id = gf_bs_read_ue_log(bs, "sps_id");
-	if (pps->sps_id >= 16) {
+	if ((pps->sps_id<0) || (pps->sps_id >= 16)) {
 		GF_LOG(GF_LOG_ERROR, GF_LOG_CODING, ("[HEVC] wrong SPS ID %d in PPS\n", pps->sps_id));
 		pps->sps_id=0;
 		return -1;
 	}
 	hevc->sps_active_idx = pps->sps_id; /*set active sps*/
@@ -9358,11 +9355,11 @@ static s32 gf_media_vvc_read_vps_bs_internal(GF_BitStream *bs, VVCState *vvc, Bo
 	VVC_VPS *vps;
 	Bool vps_default_ptl_dpb_hrd_max_tid_flag=0;
 
 	//nalu header already parsed
 	vps_id = gf_bs_read_int_log(bs, 4, "vps_id");
-	if (vps_id >= 16) return -1;
+	if ((vps_id<0) || (vps_id >= 16)) return -1;
 	if (!vps_id) {
 		GF_LOG(GF_LOG_ERROR, GF_LOG_CODING, ("[VVC] VPS ID 0 is forbidden\n"));
 		return -1;
 	}
 	vps = &vvc->vps[vps_id];
@@ -9449,15 +9446,15 @@ static s32 gf_media_vvc_read_sps_bs_internal(GF_BitStream *bs, VVCState *vvc, u8
 	u8 sps_ptl_dpb_hrd_params_present_flag;
 
 	if (vui_flag_pos) *vui_flag_pos = 0;
 
 	sps_id = gf_bs_read_int_log(bs, 4, "sps_id");
-	if (sps_id >= 16) {
+	if ((sps_id<0) || (sps_id >= 16)) {
 		return -1;
 	}
 	vps_id = gf_bs_read_int_log(bs, 4, "vps_id");
-	if (vps_id >= 16) {
+	if ((vps_id<0) || (vps_id >= 16)) {
 		return -1;
 	}
 	if (!vps_id && !vvc->vps[0].state) {
 		vvc->vps[0].state = 1;
 		vvc->vps[0].num_ptl = 1;
@@ -9655,11 +9652,11 @@ static s32 gf_media_vvc_read_pps_bs_internal(GF_BitStream *bs, VVCState *vvc)
 	if (!pps->state) {
 		pps->id = pps_id;
 		pps->state = 1;
 	}
 	pps->sps_id = gf_bs_read_int_log(bs, 4, "sps_id");
-	if (pps->sps_id >= 16) {
+	if ((pps->sps_id<0) || (pps->sps_id >= 16)) {
 		GF_LOG(GF_LOG_ERROR, GF_LOG_CODING, ("[VVC] wrong SPS ID %d in PPS\n", pps->sps_id));
 		pps->sps_id=0;
 		return -1;
 	}
 	vvc->sps_active_idx = pps->sps_id; /*set active sps*/
@@ -9723,11 +9720,11 @@ s32 vvc_parse_picture_header(GF_BitStream *bs, VVCState *vvc, VVCSliceInfo *si)
 		si->gdr_pic = gf_bs_read_int_log(bs, 1, "gdr_pic");
 	if ((si->inter_slice_allowed_flag = gf_bs_read_int_log(bs, 1, "inter_slice_allowed_flag")))
 		si->intra_slice_allowed_flag = gf_bs_read_int_log(bs, 1, "intra_slice_allowed_flag");
 
 	pps_id = gf_bs_read_ue_log(bs, "pps_id");
-	if (pps_id >= 64)
+	if ((pps_id<0) || (pps_id >= 64))
 		return -1;
 	si->pps = &vvc->pps[pps_id];
 	si->sps = &vvc->sps[si->pps->sps_id];
 	si->poc_lsb = gf_bs_read_int_log(bs, si->sps->log2_max_poc_lsb, "poc_lsb");
 

Vulnerable code: 	if (sps_id >= 32) {
		return -1;
	}
	if (sps_id < 0) {
	if (pps_id >= 255) {
	if (pps->sps_id >= 32) {
	if (pps_id >= 64)
	if (vps_id >= 16) return -1;
	if (vps_id >= 16) {
	if (pps->sps_id >= 16) {
	if (vps_id >= 16) return -1;
	if (sps_id >= 16) {
	if (vps_id >= 16) {
	if (pps->sps_id >= 16) {
	if (pps_id >= 64)
Fixed code: 	if ((sps_id < 0) || (sps_id >= 32)) {
	if ((pps_id<0) || (pps_id >= 255)) {
	if ((pps->sps_id<0) || (pps->sps_id >= 32)) {
	if ((pps_id<0) || (pps_id >= 64))
	if ((vps_id<0) || (vps_id >= 16)) return -1;
	if ((vps_id<0) || (vps_id >= 16)) {
	if ((pps->sps_id<0) || (pps->sps_id >= 16)) {
	if ((vps_id<0) || (vps_id >= 16)) return -1;
	if ((sps_id<0) || (sps_id >= 16)) {
	if ((vps_id<0) || (vps_id >= 16)) {
	if ((pps->sps_id<0) || (pps->sps_id >= 16)) {
	if ((pps_id<0) || (pps_id >= 64))