diff --git a/src/console.c b/src/console.c index 50ecfec9c..f6d1fc2a2 100644 --- a/src/console.c +++ b/src/console.c @@ -1737,6 +1737,8 @@ static void CON_DrawBackpic(void) // Cache the patch. con_backpic = W_CachePatchNum(piclump, PU_PATCH); + if (con_backpic == NULL) + return; // Center the backpic, and draw a vertically cropped patch. w = con_backpic->width * vid.dup; diff --git a/src/hardware/hw_cache.c b/src/hardware/hw_cache.c index b71bf7007..d402fbadc 100644 --- a/src/hardware/hw_cache.c +++ b/src/hardware/hw_cache.c @@ -483,10 +483,13 @@ static void HWR_GenerateTexture(INT32 texnum, GLMapTexture_t *grtex, GLMipmap_t realpatch = W_CachePatchNumPwad(wadnum, lumpnum, PU_PATCH); } - HWR_DrawTexturePatchInCache(mipmap, blockwidth, blockheight, texture, patch, realpatch); + if (realpatch != NULL) + { + HWR_DrawTexturePatchInCache(mipmap, blockwidth, blockheight, texture, patch, realpatch); - if (free_patch) - Patch_Free(realpatch); + if (free_patch) + Patch_Free(realpatch); + } } //Hurdler: not efficient at all but I don't remember exactly how HWR_DrawPatchInCache works :( if (format2bpp(mipmap->format)==4) diff --git a/src/r_patch.c b/src/r_patch.c index 19e66404f..cdb6f299a 100644 --- a/src/r_patch.c +++ b/src/r_patch.c @@ -31,14 +31,12 @@ patch_t *Patch_Create(INT16 width, INT16 height) return patch; } -patch_t *Patch_CreateFromDoomPatch(softwarepatch_t *source) +patch_t *Patch_CreateFromDoomPatch(softwarepatch_t *source, size_t insize) { - patch_t *patch = Patch_Create(0, 0); - if (!source) - return patch; + if (!Picture_CheckIfDoomPatch(source, insize)) + return NULL; - patch->width = SHORT(source->width); - patch->height = SHORT(source->height); + patch_t *patch = Patch_Create(SHORT(source->width), SHORT(source->height)); patch->leftoffset = SHORT(source->leftoffset); patch->topoffset = SHORT(source->topoffset); diff --git a/src/r_patch.h b/src/r_patch.h index ff76254e8..d47329421 100644 --- a/src/r_patch.h +++ b/src/r_patch.h @@ -19,7 +19,7 @@ // Patch functions patch_t *Patch_Create(INT16 width, INT16 height); -patch_t *Patch_CreateFromDoomPatch(softwarepatch_t *source); +patch_t *Patch_CreateFromDoomPatch(softwarepatch_t *source, size_t insize); void Patch_CalcDataSizes(softwarepatch_t *source, size_t *total_pixels, size_t *total_posts); void Patch_MakeColumns(softwarepatch_t *source, size_t num_columns, INT16 width, UINT8 *pixels, column_t *columns, post_t *posts, boolean flip); void Patch_Free(patch_t *patch); diff --git a/src/r_picformats.c b/src/r_picformats.c index a45c143b0..be4e03120 100644 --- a/src/r_picformats.c +++ b/src/r_picformats.c @@ -87,7 +87,12 @@ void *Picture_Convert( (void)insize; if (Picture_IsPatchFormat(outformat)) - return Picture_PatchConvert(informat, picture, outformat, outsize, inwidth, inheight, inleftoffset, intopoffset, flags); + { + void *converted = Picture_PatchConvert(informat, insize, picture, outformat, outsize, inwidth, inheight, inleftoffset, intopoffset, flags); + if (converted == NULL) + I_Error("Picture_Convert: conversion to patch did not result in a valid graphic"); + return converted; + } else if (Picture_IsFlatFormat(outformat)) return Picture_FlatConvert(informat, picture, outformat, outsize, inwidth, intopoffset, flags); else @@ -235,8 +240,8 @@ static void *WritePatchPixel_i8o8(void *ptr, void *input) * \return A pointer to the converted picture. */ void *Picture_PatchConvert( - pictureformat_t informat, void *picture, pictureformat_t outformat, - size_t *outsize, + pictureformat_t informat, size_t insize, void *picture, + pictureformat_t outformat, size_t *outsize, INT32 inwidth, INT32 inheight, INT32 inleftoffset, INT32 intopoffset, pictureflags_t flags) { @@ -245,7 +250,7 @@ void *Picture_PatchConvert( { if (outsize != NULL) *outsize = sizeof(patch_t); - return Patch_CreateFromDoomPatch(picture); + return Patch_CreateFromDoomPatch(picture, insize); } INT32 outbpp = Picture_FormatBPP(outformat); @@ -799,47 +804,44 @@ boolean Picture_IsFlatFormat(pictureformat_t format) } /** Returns true if the lump is a valid Doom patch. - * PICFMT_DOOMPATCH only. * * \param patch Input patch. * \param picture Input patch size. - * \return True if the input patch is valid. + * \return True if the input patch is valid, false if not. */ boolean Picture_CheckIfDoomPatch(softwarepatch_t *patch, size_t size) { - INT16 width, height; - boolean result; - - // minimum length of a valid Doom patch - if (size < 13) + // Does not meet minimum size requirements + if (size < MIN_PATCH_LUMP_SIZE) return false; - width = SHORT(patch->width); - height = SHORT(patch->height); - result = (height > 0 && height <= 16384 && width > 0 && width <= 16384); + INT16 width = SHORT(patch->width); + INT16 height = SHORT(patch->height); - if (result) + // 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 + // check the column directory for extra security. All columns + // must begin after the column directory, and none of them must + // point past the end of the patch. + for (INT16 x = 0; x < width; x++) { - // The dimensions seem like they might be valid for a patch, so - // check the column directory for extra security. All columns - // must begin after the column directory, and none of them must - // point past the end of the patch. - INT16 x; + UINT32 ofs = LONG(patch->columnofs[x]); - for (x = 0; x < width; x++) + // Need one byte for an empty column (but there's patches that don't know that!) + if (ofs < FIRST_PATCH_LUMP_COLUMN(width) || (size_t)ofs >= 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) - { - result = false; - break; - } + return false; } } - return result; + return true; } /** Converts a texture to a flat. @@ -901,12 +903,13 @@ 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 - return (memcmp(&d[0], "\x89\x50\x4e\x47\x0d\x0a\x1a\x0a", 8) == 0); + return (memcmp(&d[0], "\x89\x50\x4e\x47\x0d\x0a\x1a\x0a", PNG_HEADER_SIZE) == 0); } #ifndef NO_PNG_LUMPS @@ -1373,7 +1376,9 @@ void *Picture_PNGConvert( } // Now, convert it! - converted = Picture_PatchConvert(informat, flat, outformat, outsize, (INT32)width, (INT32)height, *leftoffset, *topoffset, flags); + converted = Picture_PatchConvert(informat, flatsize, flat, outformat, outsize, (INT32)width, (INT32)height, *leftoffset, *topoffset, flags); + if (converted == NULL) + I_Error("Picture_PNGConvert: conversion to patch did not result in a valid graphic"); Z_Free(flat); return converted; } diff --git a/src/r_picformats.h b/src/r_picformats.h index 123dda976..b1e957b10 100644 --- a/src/r_picformats.h +++ b/src/r_picformats.h @@ -55,6 +55,29 @@ enum PICDEPTH_32BPP = 32 }; +// Maximum allowed dimensions for a patch +#define MAX_PATCH_DIMENSIONS 8192 + +// Minimum amount of bytes required for a valid patch lump header +#define MIN_PATCH_LUMP_HEADER_SIZE ((sizeof(INT16) * 4) + sizeof(INT32)) + +// Minimum length of a valid Doom patch lump +// This is the size of a 1x1 patch. +#define MIN_PATCH_LUMP_SIZE (MIN_PATCH_LUMP_HEADER_SIZE + 1) + +// Gets the offset to the very first column in a patch lump +#define FIRST_PATCH_LUMP_COLUMN(width) ((sizeof(INT16) * 4) + ((width) * sizeof(INT32))) + +// Checks if the size of a lump is valid for a patch, given a certain width +#define VALID_PATCH_LUMP_SIZE(lumplen, width) ((lumplen) >= FIRST_PATCH_LUMP_COLUMN(width)) + +// 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, @@ -62,8 +85,8 @@ void *Picture_Convert( pictureflags_t flags); void *Picture_PatchConvert( - pictureformat_t informat, void *picture, pictureformat_t outformat, - size_t *outsize, + pictureformat_t informat, size_t insize, void *picture, + pictureformat_t outformat, size_t *outsize, INT32 inwidth, INT32 inheight, INT32 inleftoffset, INT32 intopoffset, pictureflags_t flags); void *Picture_FlatConvert( @@ -104,9 +127,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/r_textures.c b/src/r_textures.c index 4dc6bbae4..ee284ec83 100644 --- a/src/r_textures.c +++ b/src/r_textures.c @@ -430,6 +430,10 @@ UINT8 *R_GenerateTexture(size_t texnum) realpatch = W_CachePatchNumPwad(wadnum, lumpnum, PU_PATCH); } + // Well, it's not valid... + if (realpatch == NULL) + continue; + x1 = patch->originx; width = realpatch->width; height = realpatch->height; diff --git a/src/w_wad.c b/src/w_wad.c index 50ba622a9..f7d880e2b 100644 --- a/src/w_wad.c +++ b/src/w_wad.c @@ -1695,12 +1695,75 @@ 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[MIN_PATCH_LUMP_HEADER_SIZE]; + + 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 < MIN_PATCH_LUMP_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 12 bytes + W_ReadLumpHeaderPwad(wadnum, lumpnum, header, sizeof(header), 0); + + softwarepatch_t patch; + memcpy(&patch, header, sizeof(header)); + + INT16 width = SHORT(patch.width); + INT16 height = SHORT(patch.height); + + // 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... But check at least the first column. + UINT32 ofs = LONG(patch.columnofs[0]); + + // Need one byte for an empty column (but there's patches that don't know that!) + if (ofs < FIRST_PATCH_LUMP_COLUMN(width) || (size_t)ofs >= lumplen) + { + return false; + } + + 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 +1778,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; } } @@ -2383,9 +2410,12 @@ static void *W_GetPatchPwad(UINT16 wad, UINT16 lump, INT32 tag) #endif } - dest = Patch_CreateFromDoomPatch(ptr); + dest = Patch_CreateFromDoomPatch(ptr, len); Z_Free(ptr); + if (dest == NULL) + return NULL; + Z_ChangeTag(dest, tag); Z_SetUser(dest, &lumpcache[lump]); } @@ -2403,7 +2433,7 @@ void *W_CachePatchNumPwad(UINT16 wad, UINT16 lump, INT32 tag) patch_t *patch = W_GetPatchPwad(wad, lump, tag); #ifdef HWRENDER - if (rendermode == render_opengl) + if (patch != NULL && rendermode == render_opengl) Patch_CreateGL(patch); #endif