From 78d0bcae46c1c79ef35d7b32edcaeb6b8ee75e01 Mon Sep 17 00:00:00 2001
From: ggn <ggn@atari.org>
Date: Mon, 19 Dec 2022 13:09:06 +0200
Subject: [PATCH] Rewrote .incbin's parsing and checks for position and size to
 increase robustness

---
 direct.c | 77 +++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 46 insertions(+), 31 deletions(-)

diff --git a/direct.c b/direct.c
index 65a119d..24c3d93 100644
--- a/direct.c
+++ b/direct.c
@@ -621,32 +621,40 @@ allright:
 
 	tok += 2;
 
+	size = lseek(fd, 0L, SEEK_END);
+	pos = lseek(fd, 0L, SEEK_SET);
+
 	if (*tok != EOL)
 	{
-		// Check size parameter (can be omitted)
-		if (*tok++ == ',')
+		// Parse size and position parameters
+		uint64_t requested_size = -1;	// -1 means "not set" for these two
+		if (*tok++ != ',')
+		{
+			close(fd);
+			return error("expected comma after incbin filename");
+		}
+		if (tok != EOL)
 		{
 			if (*tok != ',')
 			{
-				if (abs_expr(&size) != OK)
+				if (abs_expr(&requested_size) != OK)
 				{
 					close(fd);
 					return ERROR;
 				}
-				if ((int64_t)size <= 0)
+				if ((int64_t)requested_size <= 0 || requested_size > size)
 				{
+					close(fd);
 					return error("invalid incbin size requested");
 				}
 			}
-			else
-				size = lseek(fd, 0L, SEEK_END);
-		}
-
-		// Check offset parameter (can be omitted)
-		if (*tok != EOL)
-		{
-			if (*tok++ == ',')
+			if (*tok != EOL)
 			{
+				if (*tok++ != ',')
+				{
+					close(fd);
+					return error("expected comma after size parameter");
+				}
 				if (*tok != EOL)
 				{
 					if (abs_expr(&pos) != OK)
@@ -654,30 +662,37 @@ allright:
 						close(fd);
 						return ERROR;
 					}
-
-					lseek(fd, pos, SEEK_SET);
-					if ((int64_t)(size - pos) < 0)
+					if ((int64_t)pos <= 0 || pos > size)
 					{
-						return error("requested incbin size out of range");
+						close(fd);
+						return error("invalid incbin position requested");
 					}
 				}
-				else
-				{
-					// offset parameter omitted, so it's 0
-					pos = lseek(fd, 0L, SEEK_SET);
-				}
 			}
-			else
-				return error(comma_error);
+
+			if (*tok != EOL)
+			{
+				close(fd);
+				return error("extra characters following incbin");
+			}
 		}
-		else
-			pos = lseek(fd, 0L, SEEK_SET);
-	}
-	else
-	{
-		// size & pos not given, so assume offset of 0 and all of the binary
-		size = lseek(fd, 0L, SEEK_END);
-		pos = lseek(fd, 0L, SEEK_SET);
+
+		// Adjust size if the user didn't specify it via the parameter
+		if (requested_size == -1)
+		{
+			requested_size = size - pos;
+		}
+		
+		// Are we going to read past the end of the file?
+		if (pos + requested_size > size)
+		{
+			close(fd);
+			return error("invalid combination of incbin position and size");
+		}
+		size = requested_size;
+
+		// All checks passed, let's seek to where the user requested, otherwise at file start
+		lseek(fd, pos, SEEK_SET);
 	}
 
 	chcheck(size);
-- 
2.37.2