From 161dab91e7e1690a77a11ee29cd5e75ea781885c Mon Sep 17 00:00:00 2001
From: Lactozilla <jp6781615@gmail.com>
Date: Tue, 28 Jan 2025 22:21:20 -0300
Subject: [PATCH] Check if a lump is a valid patch when looking for one by name
---
src/r_defs.h | 4 ++
src/r_picformats.c | 17 +++++---
src/r_picformats.h | 13 +++++--
src/w_wad.c | 96 +++++++++++++++++++++++++++-------------------
4 files changed, 82 insertions(+), 48 deletions(-)
diff --git a/src/r_defs.h b/src/r_defs.h
index eac3e2d1f3..121a17dc0c 100644
--- a/src/r_defs.h
+++ b/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};
diff --git a/src/r_picformats.c b/src/r_picformats.c
index 22ae1dde83..d58d5abbc7 100644
--- a/src/r_picformats.c
+++ b/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
diff --git a/src/r_picformats.h b/src/r_picformats.h
index 273af43760..4098591367 100644
--- a/src/r_picformats.h
+++ b/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
diff --git a/src/w_wad.c b/src/w_wad.c
index fd5f7d004a..87e13ed0f5 100644
--- a/src/w_wad.c
+++ b/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;
}
}
--
GitLab