From 45922f80d1e35aadc9da6e20fa32fa78c1044dfd Mon Sep 17 00:00:00 2001 From: Sryder Date: Sun, 23 Jun 2019 12:26:52 +0100 Subject: [PATCH 1/5] Don't read from a per-map COLORMAP if it is too big. Could this be changed to only read the first so many bytes? --- src/r_data.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/r_data.c b/src/r_data.c index a21ba49ae..bf570e3ab 100644 --- a/src/r_data.c +++ b/src/r_data.c @@ -1073,6 +1073,15 @@ void R_ReInitColormaps(UINT16 num) lump = W_GetNumForName(colormap); if (lump == LUMPERROR) lump = W_GetNumForName("COLORMAP"); + else + { + if (W_LumpLength(lump) > W_LumpLength(W_GetNumForName("COLORMAP"))) + { + CONS_Alert(CONS_WARNING, "%s lump size is too big, using COLORMAP.\n", colormap); + lump = W_GetNumForName("COLORMAP"); + } + } + W_ReadLump(lump, colormaps); // Init Boom colormaps. From bc254d9cf7ca36985481f988af0e8daa4741c7cd Mon Sep 17 00:00:00 2001 From: Sryder Date: Sun, 23 Jun 2019 13:48:29 +0100 Subject: [PATCH 2/5] Kill Texture SOC feature. As far as I know it's basically unused, and the strstr is inherently unsafe because there's no guarantee that a patch's contents are NULL terminated. --- src/r_data.c | 49 +++++++++++++++++++------------------------------ 1 file changed, 19 insertions(+), 30 deletions(-) diff --git a/src/r_data.c b/src/r_data.c index bf570e3ab..c50cbf209 100644 --- a/src/r_data.c +++ b/src/r_data.c @@ -484,42 +484,31 @@ void R_LoadTextures(void) { patchlump = W_CacheLumpNumPwad((UINT16)w, texstart + j, PU_CACHE); - // Then, check the lump directly to see if it's a texture SOC, - // and if it is, load it using dehacked instead. - if (strstr((const char *)patchlump, "TEXTURE")) - { - CONS_Alert(CONS_WARNING, "%s is a Texture SOC.\n", W_CheckNameForNumPwad((UINT16)w,texstart+j)); - Z_Unlock(patchlump); - DEH_LoadDehackedLumpPwad((UINT16)w, texstart + j); - } - else - { - //CONS_Printf("\n\"%s\" is a single patch, dimensions %d x %d",W_CheckNameForNumPwad((UINT16)w,texstart+j),patchlump->width, patchlump->height); - texture = textures[i] = Z_Calloc(sizeof(texture_t) + sizeof(texpatch_t), PU_STATIC, NULL); + //CONS_Printf("\n\"%s\" is a single patch, dimensions %d x %d",W_CheckNameForNumPwad((UINT16)w,texstart+j),patchlump->width, patchlump->height); + texture = textures[i] = Z_Calloc(sizeof(texture_t) + sizeof(texpatch_t), PU_STATIC, NULL); - // Set texture properties. - M_Memcpy(texture->name, W_CheckNameForNumPwad((UINT16)w, texstart + j), sizeof(texture->name)); - texture->width = SHORT(patchlump->width); - texture->height = SHORT(patchlump->height); - texture->patchcount = 1; - texture->holes = false; + // Set texture properties. + M_Memcpy(texture->name, W_CheckNameForNumPwad((UINT16)w, texstart + j), sizeof(texture->name)); + texture->width = SHORT(patchlump->width); + texture->height = SHORT(patchlump->height); + texture->patchcount = 1; + texture->holes = false; - // Allocate information for the texture's patches. - patch = &texture->patches[0]; + // Allocate information for the texture's patches. + patch = &texture->patches[0]; - patch->originx = patch->originy = 0; - patch->wad = (UINT16)w; - patch->lump = texstart + j; + patch->originx = patch->originy = 0; + patch->wad = (UINT16)w; + patch->lump = texstart + j; - Z_Unlock(patchlump); + Z_Unlock(patchlump); - k = 1; - while (k << 1 <= texture->width) - k <<= 1; + k = 1; + while (k << 1 <= texture->width) + k <<= 1; - texturewidthmask[i] = k - 1; - textureheight[i] = texture->height << FRACBITS; - } + texturewidthmask[i] = k - 1; + textureheight[i] = texture->height << FRACBITS; } } } From bb9b1b3b1f0f133feef8395cefa3a9cc3939a6e1 Mon Sep 17 00:00:00 2001 From: Sryder Date: Sun, 23 Jun 2019 13:49:39 +0100 Subject: [PATCH 3/5] Change COLORMAP lump size check to be exact A lower size could technically be valid, but could easily run into strange issues. --- src/r_data.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/r_data.c b/src/r_data.c index c50cbf209..be7a5dc98 100644 --- a/src/r_data.c +++ b/src/r_data.c @@ -1064,9 +1064,9 @@ void R_ReInitColormaps(UINT16 num) lump = W_GetNumForName("COLORMAP"); else { - if (W_LumpLength(lump) > W_LumpLength(W_GetNumForName("COLORMAP"))) + if (W_LumpLength(lump) != W_LumpLength(W_GetNumForName("COLORMAP"))) { - CONS_Alert(CONS_WARNING, "%s lump size is too big, using COLORMAP.\n", colormap); + CONS_Alert(CONS_WARNING, "%s lump size does not match COLORMAP, using COLORMAP instead.\n", colormap); lump = W_GetNumForName("COLORMAP"); } } From 5f339fc2a922aa00a94027e885c29173e43edac7 Mon Sep 17 00:00:00 2001 From: Sryder Date: Sun, 23 Jun 2019 14:52:49 +0100 Subject: [PATCH 4/5] Don't overlap strncpy in WAD file load --- src/w_wad.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/w_wad.c b/src/w_wad.c index 8d96449f1..e18c5a084 100644 --- a/src/w_wad.c +++ b/src/w_wad.c @@ -149,9 +149,15 @@ FILE *W_OpenWadFile(const char **filename, boolean useerrors) { FILE *handle; - strncpy(filenamebuf, *filename, MAX_WADPATH); - filenamebuf[MAX_WADPATH - 1] = '\0'; - *filename = filenamebuf; + // Officially, strncpy should not have overlapping buffers, since W_VerifyNMUSlumps is called after this, and it + // changes filename to point at filenamebuf, it would technically be doing that. I doubt any issue will occur since + // they point to the same location, but it's better to be safe and this is a simple change. + if (filenamebuf != *filename) + { + strncpy(filenamebuf, *filename, MAX_WADPATH); + filenamebuf[MAX_WADPATH - 1] = '\0'; + *filename = filenamebuf; + } // open wad file if ((handle = fopen(*filename, "rb")) == NULL) From 8a778a407001c6cf861c87ecfedbb8ed8f09860d Mon Sep 17 00:00:00 2001 From: Sryder Date: Sun, 23 Jun 2019 15:02:32 +0100 Subject: [PATCH 5/5] Simply truncate the per-map COLORMAP lump instead of not reading it at all. Keep the warning though. --- src/r_data.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/r_data.c b/src/r_data.c index be7a5dc98..29a9c52bb 100644 --- a/src/r_data.c +++ b/src/r_data.c @@ -1054,6 +1054,7 @@ void R_ReInitColormaps(UINT16 num) { char colormap[9] = "COLORMAP"; lumpnum_t lump; + const lumpnum_t basecolormaplump = W_GetNumForName(colormap); if (num > 0 && num <= 10000) snprintf(colormap, 8, "CLM%04u", num-1); @@ -1061,17 +1062,16 @@ void R_ReInitColormaps(UINT16 num) // Load in the light tables, now 64k aligned for smokie... lump = W_GetNumForName(colormap); if (lump == LUMPERROR) - lump = W_GetNumForName("COLORMAP"); + lump = basecolormaplump; else { - if (W_LumpLength(lump) != W_LumpLength(W_GetNumForName("COLORMAP"))) + if (W_LumpLength(lump) != W_LumpLength(basecolormaplump)) { - CONS_Alert(CONS_WARNING, "%s lump size does not match COLORMAP, using COLORMAP instead.\n", colormap); - lump = W_GetNumForName("COLORMAP"); + CONS_Alert(CONS_WARNING, "%s lump size does not match COLORMAP, results may be unexpected.\n", colormap); } } - W_ReadLump(lump, colormaps); + W_ReadLumpHeader(lump, colormaps, W_LumpLength(basecolormaplump), 0U); // Init Boom colormaps. R_ClearColormaps();