CVE-2023-1654: Uncontrolled Resource Consumption (CWE-400)
value: Denial of Service in GitHub repository gpac/gpac prior to 2.4.0.
CWE description: The product does not properly control the allocation and maintenance of a limited resource, thereby enabling an actor to influence the amount of resources consumed, eventually leading to the exhaustion of available resources.
CVSS v3 base score: 7.8
Published: 2023-03-27T16:15Z
Repository: https://github.com/gpac/gpac
Language: C
Files changed: src/filter_core/filter_pid.c
Commit date: 2023-03-27 11:58:57 +0200
Commit message: fixed #2429

Diff: diff --git a/src/filter_core/filter_pid.c b/src/filter_core/filter_pid.c
index 020ec8f32526..5e14ab36f668 100644
--- a/src/filter_core/filter_pid.c
+++ b/src/filter_core/filter_pid.c
@@ -60,17 +60,21 @@ void gf_filter_pid_inst_del(GF_FilterPidInst *pidinst)
  	gf_fq_del(pidinst->packets, (gf_destruct_fun) pcki_del);
 	gf_mx_del(pidinst->pck_mx);
 	gf_list_del(pidinst->pck_reassembly);
 	if (pidinst->props) {
 		assert(pidinst->props->reference_count);
-		if (safe_int_dec(&pidinst->props->reference_count) == 0) {
-			//see \ref gf_filter_pid_merge_properties_internal for mutex
-			gf_mx_p(pidinst->pid->filter->tasks_mx);
-			gf_list_del_item(pidinst->pid->properties, pidinst->props);
-			gf_mx_v(pidinst->pid->filter->tasks_mx);
-			gf_props_del(pidinst->props);
+		gf_mx_p(pidinst->pid->filter->tasks_mx);
+		//not in parent pid, may happen when reattaching a pid inst to a different pid
+		//in this case do NOT delete the props
+		if (gf_list_find(pidinst->pid->properties, pidinst->props)>=0) {
+			if (safe_int_dec(&pidinst->props->reference_count) == 0) {
+				//see \ref gf_filter_pid_merge_properties_internal for mutex
+				gf_list_del_item(pidinst->pid->properties, pidinst->props);
+				gf_props_del(pidinst->props);
+			}
 		}
+		gf_mx_v(pidinst->pid->filter->tasks_mx);
 	}
 	gf_free(pidinst);
 }
 
 static GF_FilterPidInst *gf_filter_pid_inst_new(GF_Filter *filter, GF_FilterPid *pid)
@@ -1042,12 +1046,19 @@ static GF_Err gf_filter_pid_configure(GF_Filter *filter, GF_FilterPid *pid, GF_P
 						GF_Filter *target = filter->swap_pidinst_dst->filter;
 						assert(target);
 						if (!target->detached_pid_inst) {
 							target->detached_pid_inst = gf_list_new();
 						}
-						//detach props but don't delete them
+						//detach props
 						if (filter->swap_pidinst_dst->props) {
+							GF_FilterPidInst *swap_pidi = filter->swap_pidinst_dst;
+							if (safe_int_dec(&swap_pidi->props->reference_count)==0) {
+								gf_mx_p(swap_pidi->pid->filter->tasks_mx);
+								gf_list_del_item(swap_pidi->pid->properties, pidinst->props);
+								gf_mx_v(swap_pidi->pid->filter->tasks_mx);
+								gf_props_del(pidinst->props);
+							}
 							filter->swap_pidinst_dst->props = NULL;
 						}
 						filter->swap_pidinst_dst->pid = NULL;
 						if (gf_list_find(target->detached_pid_inst, filter->swap_pidinst_dst)<0)
 							gf_list_add(target->detached_pid_inst, filter->swap_pidinst_dst);
@@ -1306,11 +1317,11 @@ void gf_filter_pid_detach_task(GF_FSTask *task)
 	safe_int_dec(&pid->filter->detach_pid_tasks_pending);
 
 	//first connection of this PID to this filter
 	if (!pidinst) {
 		GF_LOG(GF_LOG_ERROR, GF_LOG_FILTER, ("Trying to detach PID %s not present in filter %s inputs\n",  pid->name, filter->name));
-		//assert(!new_chain_input->swap_pidinst_dst);
+		//when swaping encoder, we may have swap_pidinst_dst not NULL so only check swap_pidinst_src
 		assert(!new_chain_input->swap_pidinst_src);
 		new_chain_input->swap_needs_init = GF_FALSE;
 		return;
 	}
 
@@ -5941,11 +5952,11 @@ static GF_Err gf_filter_pid_merge_properties_internal(GF_FilterPid *dst_pid, GF_
 			assert(pidi->props);
 			safe_int_inc(&pidi->props->reference_count);
 		}
 		src_props = pidi->props;
 	}
-	//move to rela pid
+	//move to real pid
 	src_pid = src_pid->pid;
 	//this is a copy props on output pid
 	if (!src_props) {
 		//our list is not thread-safe, so we must lock the filter when destroying the props
 		//otherwise gf_list_last() (this caller) might use the last entry while another thread sets this last entry to NULL

Vulnerable code: 		if (safe_int_dec(&pidinst->props->reference_count) == 0) {
			//see \ref gf_filter_pid_merge_properties_internal for mutex
			gf_mx_p(pidinst->pid->filter->tasks_mx);
			gf_list_del_item(pidinst->pid->properties, pidinst->props);
			gf_mx_v(pidinst->pid->filter->tasks_mx);
			gf_props_del(pidinst->props);
						//detach props but don't delete them
		//assert(!new_chain_input->swap_pidinst_dst);
	//move to rela pid
Fixed code: 		gf_mx_p(pidinst->pid->filter->tasks_mx);
		//not in parent pid, may happen when reattaching a pid inst to a different pid
		//in this case do NOT delete the props
		if (gf_list_find(pidinst->pid->properties, pidinst->props)>=0) {
			if (safe_int_dec(&pidinst->props->reference_count) == 0) {
				//see \ref gf_filter_pid_merge_properties_internal for mutex
				gf_list_del_item(pidinst->pid->properties, pidinst->props);
				gf_props_del(pidinst->props);
			}
		gf_mx_v(pidinst->pid->filter->tasks_mx);
						//detach props
							GF_FilterPidInst *swap_pidi = filter->swap_pidinst_dst;
							if (safe_int_dec(&swap_pidi->props->reference_count)==0) {
								gf_mx_p(swap_pidi->pid->filter->tasks_mx);
								gf_list_del_item(swap_pidi->pid->properties, pidinst->props);
								gf_mx_v(swap_pidi->pid->filter->tasks_mx);
								gf_props_del(pidinst->props);
							}
		//when swaping encoder, we may have swap_pidinst_dst not NULL so only check swap_pidinst_src
	//move to real pid