**[FIX]** fix infinite loop in MP4 file type detector and processor (#…

…1566)

* Update stream_functions.c: fix MP4 file type detector

On bad inputs containing e.g. the following sequence of bytes within the first 1MiB "ff ff ff ff 6d 65 74 61" `detect_stream_type` was executing an infinite loop because "ff ff ff ff" was interpreted as a length of the candidate "meta" MP4 box, caused the size_t overflow inside `isValidMP4Box` which pointed `nextBoxLocation` to the previous byte and the execution flow processed the same "meta" again.

* Update CHANGES.TXT

* Treat a candidate MP4 box as invalid instead of bailing out

* Fix stuck mp4 processing in `process_avc_sample`

On corrupted inputs it could read data past the sample end and also get stuck in an infinite loop.

* Fix the stats code to not count zero-sized NALs and avoid dereferencing memory past the NAL end

* Add comment.

* Format changes

docs/CHANGES.TXT

@@ -21,6 +21,7 @@
 - Fix: Repeated values for enums
 - Cleanup: Remove the (unmaintained) Nuklear GUI code
 - Cleanup: Reduce the amount of Windows build options in the project file
+- Fix: infinite loop in MP4 file type detector.
 
 0.94 (2021-12-14)
 -----------------

src/lib_ccx/mp4.c

@@ -51,6 +51,16 @@ static int process_avc_sample(struct lib_ccx_ctx *ctx, u32 timescale, GF_AVCConf
 	{
 		u32 nal_length;
 
+		if (i + c->nal_unit_size > s->dataLength)
+		{
+			mprint("Corrupted packet detected in process_avc_sample. dataLength "
+			       "%u is less than index %u + nal_unit_size %u. Ignoring.\n",
+			       s->dataLength, i, c->nal_unit_size);
+			// The packet is likely corrupted, it's unsafe to read this many bytes
+			// even to detect the length of the next `nal`. Ignoring this error,
+			// hopefully the outer loop in `process_avc_track` can recover.
+			return status;
+		}
 		switch (c->nal_unit_size)
 		{
 			case 1:
@@ -63,15 +73,28 @@ static int process_avc_sample(struct lib_ccx_ctx *ctx, u32 timescale, GF_AVCConf
 				nal_length = bswap32(*(long *)&s->data[i]);
 				break;
 		}
+		const u32 previous_index = i;
 		i += c->nal_unit_size;
+		if (i + nal_length <= previous_index || i + nal_length > s->dataLength)
+		{
+			mprint("Corrupted sample detected in process_avc_sample. dataLength %u "
+			       "is less than index %u + nal_unit_size %u + nal_length %u. Ignoring.\n",
+			       s->dataLength, previous_index, c->nal_unit_size, nal_length);
+			// The packet is likely corrupted, it's unsafe to procell nal_length bytes
+			// because they are past the sample end. Ignoring this error, hopefully
+			// the outer loop in `process_avc_track` can recover.
+			return status;
+		}
 
 		s_nalu_stats.total += 1;
-		s_nalu_stats.type[s->data[i] & 0x1F] += 1;
-
 		temp_debug = 0;
 
 		if (nal_length > 0)
+		{
+			// s->data[i] is only relevant and safe to access here.
+			s_nalu_stats.type[s->data[i] & 0x1F] += 1;
 			do_NAL(enc_ctx, dec_ctx, (unsigned char *)&(s->data[i]), nal_length, sub);
+		}
 		i += nal_length;
 	} // outer for
 	assert(i == s->dataLength);

src/lib_ccx/stream_functions.c

@@ -99,7 +99,7 @@ void detect_stream_type(struct ccx_demuxer *ctx)
 		while (idx < ctx->startbytes_avail - 8)
 		{
 			// Check if we have a valid box
-			if (isValidMP4Box(ctx->startbytes, idx, &nextBoxLocation, &boxScore))
+			if (isValidMP4Box(ctx->startbytes, idx, &nextBoxLocation, &boxScore) && nextBoxLocation > idx)
 			{
 				idx = nextBoxLocation; // If the box is valid, a new box should be found on the next location... Not somewhere in between.
 				if (boxScore > 7)