From 6b4d4226649eebb200f1f00e0f863f39a28ea57d Mon Sep 17 00:00:00 2001 From: James R Date: Sat, 28 Nov 2020 20:00:01 -0800 Subject: [PATCH 1/2] Don't add pk3 if there are holes ZIP tools often read the final central directory, but SRB2 may not if there are multiple central directories. It's just easier to not allow "holes", or unaccounted for bytes in the file. --- src/w_wad.c | 111 ++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 85 insertions(+), 26 deletions(-) diff --git a/src/w_wad.c b/src/w_wad.c index aca530fa5..19432e937 100644 --- a/src/w_wad.c +++ b/src/w_wad.c @@ -716,7 +716,7 @@ UINT16 W_InitFile(const char *filename, boolean mainfile, boolean startup) #endif size_t packetsize; UINT8 md5sum[16]; - boolean important; + int important; if (!(refreshdirmenu & REFRESHDIR_ADDFILE)) refreshdirmenu = REFRESHDIR_NORMAL|REFRESHDIR_ADDFILE; // clean out cons_alerts that happened earlier @@ -746,10 +746,18 @@ UINT16 W_InitFile(const char *filename, boolean mainfile, boolean startup) if ((handle = W_OpenWadFile(&filename, true)) == NULL) return W_InitFileError(filename, startup); + important = W_VerifyNMUSlumps(filename); + + if (important == -1) + { + fclose(handle); + return W_InitFileError(filename, startup); + } + // Check if wad files will overflow fileneededbuffer. Only the filename part // is send in the packet; cf. // see PutFileNeeded in d_netfil.c - if ((important = !W_VerifyNMUSlumps(filename))) + if ((important = !important)) { packetsize = packetsizetally + nameonlylength(filename) + 22; @@ -1919,8 +1927,16 @@ static lumpchecklist_t folderblacklist[] = static int W_VerifyPK3 (FILE *fp, lumpchecklist_t *checklist, boolean status) { + int verified = true; + zend_t zend; zentry_t zentry; + zlentry_t zlentry; + + long file_size;/* size of zip file */ + long data_size;/* size of data inside zip file */ + + long old_position; UINT16 numlumps; size_t i; @@ -1936,6 +1952,8 @@ W_VerifyPK3 (FILE *fp, lumpchecklist_t *checklist, boolean status) // Central directory bullshit fseek(fp, 0, SEEK_END); + file_size = ftell(fp); + if (!ResFindSignature(fp, pat_end, max(0, ftell(fp) - (22 + 65536)))) return true; @@ -1943,6 +1961,8 @@ W_VerifyPK3 (FILE *fp, lumpchecklist_t *checklist, boolean status) if (fread(&zend, 1, sizeof zend, fp) < sizeof zend) return true; + data_size = sizeof zend; + numlumps = zend.entries; fseek(fp, zend.cdiroffset, SEEK_SET); @@ -1957,40 +1977,79 @@ W_VerifyPK3 (FILE *fp, lumpchecklist_t *checklist, boolean status) if (memcmp(zentry.signature, pat_central, 4)) return true; - fullname = malloc(zentry.namelen + 1); - if (fgets(fullname, zentry.namelen + 1, fp) != fullname) - return true; - - // Strip away file address and extension for the 8char name. - if ((trimname = strrchr(fullname, '/')) != 0) - trimname++; - else - trimname = fullname; // Care taken for root files. - - if (*trimname) // Ignore directories, well kinda + if (verified == true) { - if ((dotpos = strrchr(trimname, '.')) == 0) - dotpos = fullname + strlen(fullname); // Watch for files without extension. + fullname = malloc(zentry.namelen + 1); + if (fgets(fullname, zentry.namelen + 1, fp) != fullname) + return true; - memset(lumpname, '\0', 9); // Making sure they're initialized to 0. Is it necessary? - strncpy(lumpname, trimname, min(8, dotpos - trimname)); + // Strip away file address and extension for the 8char name. + if ((trimname = strrchr(fullname, '/')) != 0) + trimname++; + else + trimname = fullname; // Care taken for root files. - if (! W_VerifyName(lumpname, checklist, status)) - return false; + if (*trimname) // Ignore directories, well kinda + { + if ((dotpos = strrchr(trimname, '.')) == 0) + dotpos = fullname + strlen(fullname); // Watch for files without extension. - // Check for directories next, if it's blacklisted it will return false - if (W_VerifyName(fullname, folderblacklist, status)) - return false; + memset(lumpname, '\0', 9); // Making sure they're initialized to 0. Is it necessary? + strncpy(lumpname, trimname, min(8, dotpos - trimname)); + + if (! W_VerifyName(lumpname, checklist, status)) + verified = false; + + // Check for directories next, if it's blacklisted it will return false + else if (W_VerifyName(fullname, folderblacklist, status)) + verified = false; + } + + free(fullname); + + // skip and ignore comments/extra fields + if (fseek(fp, zentry.xtralen + zentry.commlen, SEEK_CUR) != 0) + return true; + } + else + { + if (fseek(fp, zentry.namelen + zentry.xtralen + zentry.commlen, SEEK_CUR) != 0) + return true; } - free(fullname); + data_size += + sizeof zentry + zentry.namelen + zentry.xtralen + zentry.commlen; - // skip and ignore comments/extra fields - if (fseek(fp, zentry.xtralen + zentry.commlen, SEEK_CUR) != 0) + old_position = ftell(fp); + + if (fseek(fp, zentry.offset, SEEK_SET) != 0) return true; + + if (fread(&zlentry, 1, sizeof(zlentry_t), fp) < sizeof (zlentry_t)) + return true; + + data_size += + sizeof zlentry + zlentry.namelen + zlentry.xtralen + zlentry.compsize; + + fseek(fp, old_position, SEEK_SET); } - return true; + if (data_size < file_size) + { + const char * error = "ZIP file has holes (%ld extra bytes)\n"; + CONS_Alert(CONS_ERROR, error, (file_size - data_size)); + return -1; + } + else if (data_size > file_size) + { + const char * error = "Reported size of ZIP file contents exceeds file size (%ld extra bytes)\n"; + CONS_Alert(CONS_ERROR, error, (data_size - file_size)); + return -1; + } + else + { + return verified; + } } // Note: This never opens lumps themselves and therefore doesn't have to From 445d0407959ae36f93f7d7265ec5e36093782e8f Mon Sep 17 00:00:00 2001 From: James R Date: Sat, 28 Nov 2020 20:51:21 -0800 Subject: [PATCH 2/2] Don't print W_VerifyFile errors more than once --- src/d_main.c | 6 +----- src/d_netcmd.c | 8 +++++++- src/w_wad.c | 23 +++++++++++++++++------ src/w_wad.h | 2 +- 4 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/d_main.c b/src/d_main.c index 1045d4d99..ace1b5ed4 100644 --- a/src/d_main.c +++ b/src/d_main.c @@ -998,7 +998,7 @@ static void IdentifyVersion(void) #define MUSICTEST(str) \ {\ const char *musicpath = va(pandf,srb2waddir,str);\ - int ms = W_VerifyNMUSlumps(musicpath); \ + int ms = W_VerifyNMUSlumps(musicpath, false); \ if (ms == 1) \ D_AddFile(startupwadfiles, musicpath); \ else if (ms == 0) \ @@ -1187,11 +1187,7 @@ void D_SRB2Main(void) const char *s = M_GetNextParm(); if (s) // Check for NULL? - { - if (!W_VerifyNMUSlumps(s)) - G_SetGameModified(true); D_AddFile(startuppwads, s); - } } } } diff --git a/src/d_netcmd.c b/src/d_netcmd.c index 31c10f58a..49a465ce7 100644 --- a/src/d_netcmd.c +++ b/src/d_netcmd.c @@ -3294,7 +3294,13 @@ static void Command_Addfile(void) if (!isprint(fn[i]) || fn[i] == ';') return; - musiconly = W_VerifyNMUSlumps(fn); + musiconly = W_VerifyNMUSlumps(fn, false); + + if (musiconly == -1) + { + addedfiles[numfilesadded++] = fn; + continue; + } if (!musiconly) { diff --git a/src/w_wad.c b/src/w_wad.c index 19432e937..2429eaf92 100644 --- a/src/w_wad.c +++ b/src/w_wad.c @@ -66,6 +66,7 @@ #include "p_setup.h" // P_ScanThings #endif #include "m_misc.h" // M_MapNumber +#include "g_game.h" // G_SetGameModified #ifdef HWRENDER #include "hardware/hw_main.h" @@ -683,9 +684,9 @@ static UINT16 W_InitFileError (const char *filename, boolean exitworthy) if (exitworthy) { #ifdef _DEBUG - CONS_Error("A WAD file was not found or not valid.\nCheck the log to see which ones.\n"); + CONS_Error(va("%s was not found or not valid.\nCheck the log for more details.\n", filename)); #else - I_Error("A WAD file was not found or not valid.\nCheck the log to see which ones.\n"); + I_Error("%s was not found or not valid.\nCheck the log for more details.\n", filename); #endif } else @@ -746,12 +747,12 @@ UINT16 W_InitFile(const char *filename, boolean mainfile, boolean startup) if ((handle = W_OpenWadFile(&filename, true)) == NULL) return W_InitFileError(filename, startup); - important = W_VerifyNMUSlumps(filename); + important = W_VerifyNMUSlumps(filename, startup); if (important == -1) { fclose(handle); - return W_InitFileError(filename, startup); + return INT16_MAX; } // Check if wad files will overflow fileneededbuffer. Only the filename part @@ -819,6 +820,9 @@ UINT16 W_InitFile(const char *filename, boolean mainfile, boolean startup) return W_InitFileError(filename, startup); } + if (important && !mainfile) + G_SetGameModified(true); + // // link wad file to search files // @@ -2088,12 +2092,13 @@ static int W_VerifyFile(const char *filename, lumpchecklist_t *checklist, * be sent. * * \param filename Filename of the wad to check. + * \param exit_on_error Whether to exit upon file error. * \return 1 if file contains only music/sound lumps, 0 if it contains other * stuff (maps, sprites, dehacked lumps, and so on). -1 if there no * file exists with that filename * \author Alam Arias */ -int W_VerifyNMUSlumps(const char *filename) +int W_VerifyNMUSlumps(const char *filename, boolean exit_on_error) { // MIDI, MOD/S3M/IT/XM/OGG/MP3/WAV, WAVE SFX // ENDOOM text and palette lumps @@ -2167,7 +2172,13 @@ int W_VerifyNMUSlumps(const char *filename) {NULL, 0}, }; - return W_VerifyFile(filename, NMUSlist, false); + + int status = W_VerifyFile(filename, NMUSlist, false); + + if (status == -1) + W_InitFileError(filename, exit_on_error); + + return status; } /** \brief Generates a virtual resource used for level data loading. diff --git a/src/w_wad.h b/src/w_wad.h index 1e86eea5a..d0a86bcb4 100644 --- a/src/w_wad.h +++ b/src/w_wad.h @@ -206,6 +206,6 @@ void W_UnlockCachedPatch(void *patch); void W_VerifyFileMD5(UINT16 wadfilenum, const char *matchmd5); -int W_VerifyNMUSlumps(const char *filename); +int W_VerifyNMUSlumps(const char *filename, boolean exit_on_error); #endif // __W_WAD__