From 1baf7c60bd23ff815b64bdb70542affca199f1a1 Mon Sep 17 00:00:00 2001
From: jeanlf <jeanlf@gpac.io>
Date: Mon, 15 Jun 2020 09:52:18 +0200
Subject: [PATCH] fixed potential crash - cf #1389

---
 src/bifs/field_decode.c          | 10 +++++-
 src/scenegraph/base_scenegraph.c | 54 ++++++++++++++++----------------
 2 files changed, 36 insertions(+), 28 deletions(-)

diff --git a/src/bifs/field_decode.c b/src/bifs/field_decode.c
index e66366566..8b567dbf2 100644
--- a/src/bifs/field_decode.c
+++ b/src/bifs/field_decode.c
@@ -26,6 +26,7 @@
 
 
 #include <gpac/internal/bifs_dev.h>
+#include <gpac/scene_manager.h>
 #include "quant.h"
 #include "script.h"
 
@@ -721,9 +722,16 @@ GF_Node *gf_bifs_dec_node(GF_BifsDecoder * codec, GF_BitStream *bs, u32 NDT_Tag)
 		/*NULL node is encoded as USE with ID = all bits to 1*/
 		if (nodeID == (u32) (1<<codec->info->config.NodeIDBits))
 			return NULL;
-		//find node and return it
+		//find node
 		new_node = gf_sg_find_node(codec->current_graph, nodeID);
 
+		//check node is allowed for the given NDT
+		if (new_node && !gf_node_in_table(new_node, NDT_Tag)) {
+			GF_LOG(GF_LOG_ERROR, GF_LOG_CODING, ("[BIFS] Node %s not allowed as field/child of NDT type %d\n", gf_node_get_class_name(new_node), NDT_Tag));
+			codec->LastError = GF_SG_UNKNOWN_NODE;
+			return NULL;
+		}
+
 		if (!new_node) {
 			codec->LastError = GF_SG_UNKNOWN_NODE;
 		} else {
diff --git a/src/scenegraph/base_scenegraph.c b/src/scenegraph/base_scenegraph.c
index 077912872..8fdbb61a6 100644
--- a/src/scenegraph/base_scenegraph.c
+++ b/src/scenegraph/base_scenegraph.c
@@ -384,6 +384,7 @@ restart:
 	reg_node = sg->id_node;
 	while (reg_node) {
 		Bool ignore = 0;
+		GF_ParentList *nlist;
 		GF_Node *node = reg_node->node;
 		if (!node
 #ifndef GPAC_DISABLE_VRML
@@ -396,43 +397,42 @@ restart:
 
 		/*first replace all instances in parents by NULL WITHOUT UNREGISTERING (to avoid destroying the node).
 		This will take care of nodes referencing themselves*/
-		{
-			GF_ParentList *nlist = node->sgprivate->parents;
+		nlist = node->sgprivate->parents;
 #ifndef GPAC_DISABLE_SVG
-			type = (node->sgprivate->tag>GF_NODE_RANGE_LAST_VRML) ? 1 : 0;
+		type = (node->sgprivate->tag>GF_NODE_RANGE_LAST_VRML) ? 1 : 0;
 #endif
-			while (nlist) {
-				GF_ParentList *next = nlist->next;
+		while (nlist) {
+			GF_ParentList *next = nlist->next;
 #if 0
-				/*parent is a DEF'ed node, try to clean-up properly?*/
-				if ((nlist->node!=node) && SG_SearchForNode(sg, nlist->node) != NULL) {
-					ignore = 1;
-					break;
-				}
+			/*parent is a DEF'ed node, try to clean-up properly?*/
+			if ((nlist->node!=node) && SG_SearchForNode(sg, nlist->node) != NULL) {
+				ignore = 1;
+				break;
+			}
 #endif
 
 #ifndef GPAC_DISABLE_SVG
-				if (type) {
-					ReplaceIRINode(nlist->node, node, NULL);
-				} else
+			if (type) {
+				ReplaceIRINode(nlist->node, node, NULL);
+			} else
 #endif
-					ReplaceDEFNode(nlist->node, reg_node->node, NULL, 0);
+				ReplaceDEFNode(nlist->node, reg_node->node, NULL, 0);
 
-				/*direct cyclic reference to ourselves, make sure we update the parentList to the next entry before freeing it
-				since the next parent node could be reg_node again (reg_node->reg_node)*/
-				if (nlist->node==node) {
-					node->sgprivate->parents = next;
-				}
-				gf_free(nlist);
-				nlist = next;
+			/*direct cyclic reference to ourselves, make sure we update the parentList to the next entry before freeing it
+			since the next parent node could be reg_node again (reg_node->reg_node)*/
+			if (nlist->node==node) {
+				node->sgprivate->parents = next;
 			}
-			if (ignore) {
-				node->sgprivate->parents = nlist;
-				continue;
-			}
-
-			node->sgprivate->parents = NULL;
+			gf_free(nlist);
+			nlist = next;
 		}
+		if (ignore) {
+			node->sgprivate->parents = nlist;
+			continue;
+		}
+
+		node->sgprivate->parents = NULL;
+
 		//sg->node_registry[i-1] = NULL;
 		count = get_num_id_nodes(sg);
 		node->sgprivate->num_instances = 1;