Check if a lump is a valid patch when looking for one by name

src/r_defs.h

@@ -910,6 +910,10 @@ typedef struct
 #pragma pack()
 #endif
 
+#define MAX_PATCH_DIMENSIONS 8192
+
+#define VALID_PATCH_LUMP_SIZE(lumplen, width) ((lumplen) >= (sizeof(INT16) * 4) + ((width) * sizeof(INT32)))
+
 // Possible alpha types for a patch.
 enum patchalphastyle {AST_COPY, AST_TRANSLUCENT, AST_ADD, AST_SUBTRACT, AST_REVERSESUBTRACT, AST_MODULATE, AST_OVERLAY, AST_FOG};
 

src/r_picformats.c

@@ -811,13 +811,19 @@ boolean Picture_IsFlatFormat(pictureformat_t format)
   */
 boolean Picture_CheckIfDoomPatch(softwarepatch_t *patch, size_t size)
 {
-	// Minimum length of a valid Doom patch
-	if (size < 13)
+	// Does not meet minimum size requirements
+	if (size < PATCH_MIN_SIZE)
 		return false;
 
 	INT16 width = SHORT(patch->width);
 	INT16 height = SHORT(patch->height);
-	if (width <= 0 || height <= 0)
+
+	// Quickly check for the dimensions first.
+	if (width <= 0 || height <= 0 || width > MAX_PATCH_DIMENSIONS || height > MAX_PATCH_DIMENSIONS)
+		return false;
+
+	// Lump size makes no sense given the width
+	if (!VALID_PATCH_LUMP_SIZE(size, width))
 		return false;
 
 	// The dimensions seem like they might be valid for a patch, so
@@ -829,7 +835,7 @@ boolean Picture_CheckIfDoomPatch(softwarepatch_t *patch, size_t size)
 		UINT32 ofs = LONG(patch->columnofs[x]);
 
 		// Need one byte for an empty column (but there's patches that don't know that!)
-		if (ofs < (UINT32)width * 4 + 8 || ofs >= (UINT32)size)
+		if (ofs < ((sizeof(INT16) * 4) + (width * sizeof(INT32))) || ofs >= (UINT32)size)
 		{
 			return false;
 		}
@@ -897,8 +903,9 @@ void *Picture_TextureToFlat(size_t texnum)
   */
 boolean Picture_IsLumpPNG(const UINT8 *d, size_t s)
 {
-	if (s < 67) // https://web.archive.org/web/20230524232139/http://garethrees.org/2007/11/14/pngcrush/
+	if (s < PNG_MIN_SIZE)
 		return false;
+
 	// Check for PNG file signature using memcmp
 	// As it may be faster on CPUs with slow unaligned memory access
 	// Ref: http://www.libpng.org/pub/png/spec/1.2/PNG-Rationale.html#R.PNG-file-signature

src/r_picformats.h

@@ -55,6 +55,16 @@ enum
 	PICDEPTH_32BPP = 32
 };
 
+// Minimum length of a valid Doom patch
+#define PATCH_MIN_SIZE 13
+
+// Minimum size of a PNG file.
+// See: https://web.archive.org/web/20230524232139/http://garethrees.org/2007/11/14/pngcrush/
+#define PNG_MIN_SIZE 67
+
+// Size of a PNG header
+#define PNG_HEADER_SIZE 8
+
 void *Picture_Convert(
 	pictureformat_t informat, void *picture, pictureformat_t outformat,
 	size_t insize, size_t *outsize,
@@ -104,9 +114,6 @@ typedef struct
 	boolean available;
 } spriteinfo_t;
 
-// PNG support
-#define PNG_HEADER_SIZE 8
-
 boolean Picture_IsLumpPNG(const UINT8 *d, size_t s);
 
 #ifndef NO_PNG_LUMPS

src/w_wad.c

@@ -1695,12 +1695,64 @@ lumpnum_t W_GetNumForLongName(const char *name)
 	return i;
 }
 
+// Checks if a given lump might be a valid patch.
+// This will need to read a bit of the file, but it attempts to not load it
+// in its entirety.
+static boolean W_IsProbablyValidPatch(UINT16 wadnum, UINT16 lumpnum)
+{
+	UINT8 header[sizeof(INT16) * 4];
+
+	I_StaticAssert(sizeof(header) >= PNG_HEADER_SIZE);
+
+	// Check the file's size first
+	size_t lumplen = W_LumpLengthPwad(wadnum, lumpnum);
+
+	// Cannot be a valid Doom patch
+	if (lumplen < PATCH_MIN_SIZE)
+		return false;
+
+	// Check if it's probably a valid PNG
+	if (lumplen >= PNG_MIN_SIZE)
+	{
+		// Read the PNG's header
+		W_ReadLumpHeaderPwad(wadnum, lumpnum, header, PNG_HEADER_SIZE, 0);
+
+		if (Picture_IsLumpPNG(header, lumplen))
+		{
+			// Assume it is if the signature matches.
+			return true;
+		}
+
+		// Otherwise, we read it as a patch
+	}
+
+	// Read the first 8 bytes
+	W_ReadLumpHeaderPwad(wadnum, lumpnum, header, sizeof(header), 0);
+
+	INT16 width = ((UINT16 *)header)[0];
+	INT16 height = ((UINT16 *)header)[1];
+
+	// Lump size makes no sense given the width
+	if (!VALID_PATCH_LUMP_SIZE(lumplen, width))
+		return false;
+
+	// Check the dimensions.
+	if (width > 0 && height > 0 && width <= MAX_PATCH_DIMENSIONS && height <= MAX_PATCH_DIMENSIONS)
+	{
+		// Dimensions seem to make sense. Patch might be valid
+		return true;
+	}
+
+	// Not valid if this point was reached
+	return false;
+}
+
 //
 // Same as W_CheckNumForNamePwad, but handles namespaces.
 //
 static UINT16 W_CheckNumForPatchNamePwad(const char *name, UINT16 wad, boolean longname)
 {
-	UINT16 i, start = INT16_MAX, end = INT16_MAX;
+	UINT16 i;
 	static char uname[8 + 1] = { 0 };
 	UINT32 hash = 0;
 	lumpinfo_t *lump_p;
@@ -1715,51 +1767,15 @@ static UINT16 W_CheckNumForPatchNamePwad(const char *name, UINT16 wad, boolean l
 		hash = quickncasehash(uname, 8);
 	}
 
-	// SRB2 doesn't have a specific namespace for graphics, which means someone can do weird things
-	// like placing graphics inside a namespace it doesn't make sense for them to be in, like Sounds/ or SOC/
-	// So for now, this checks for lumps OUTSIDE of the flats namespace.
-	// When this situation changes, change the loops below to check for lumps INSIDE the namespaces to look in.
-	// TODO: cache namespace lump IDs
-	if (W_FileHasFolders(wadfiles[wad]))
-	{
-		start = W_CheckNumForFolderStartPK3("Flats/", wad, 0);
-		end = W_CheckNumForFolderEndPK3("Flats/", wad, start);
-
-		// if the start and end is the same, the folder is empty
-		if (end <= start)
-		{
-			start = INT16_MAX;
-			end = INT16_MAX;
-		}
-	}
-	else
-	{
-		start = W_CheckNumForMarkerStartPwad("F_START", wad, 0);
-		end = W_CheckNumForNamePwad("F_END", wad, start);
-		if (end != INT16_MAX)
-			end++;
-	}
-
 	lump_p = wadfiles[wad]->lumpinfo;
 
-	if (start == INT16_MAX)
-		start = wadfiles[wad]->numlumps;
-
-	for (i = 0; i < start; i++, lump_p++)
+	for (i = 0; i < wadfiles[wad]->numlumps; i++, lump_p++)
 	{
 		if ((!longname && lump_p->hash == hash && !strncmp(lump_p->name, uname, sizeof(uname) - 1))
 		|| (longname && stricmp(lump_p->longname, name) == 0))
-			return i;
-	}
-
-	if (end != INT16_MAX && start < end)
-	{
-		lump_p = wadfiles[wad]->lumpinfo + end;
-
-		for (i = end; i < wadfiles[wad]->numlumps; i++, lump_p++)
 		{
-			if ((!longname && lump_p->hash == hash && !strncmp(lump_p->name, uname, sizeof(uname) - 1))
-			|| (longname && stricmp(lump_p->longname, name) == 0))
+			// Found the patch by name, but needs to check if it is valid.
+			if (W_IsProbablyValidPatch(wad, i))
 				return i;
 		}
 	}