Merge branch 'holy-zip' into 'next'

Disallow pk3s with extra bytes

See merge request STJr/SRB2!1275

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);
-				}
 			}
 		}
 	}

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)
 		{

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
@@ -716,7 +717,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 +747,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, startup);
+
+	if (important == -1)
+	{
+		fclose(handle);
+		return INT16_MAX;
+	}
+
 	// 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;
 
@@ -811,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
 	//
@@ -1919,8 +1931,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 +1956,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 +1965,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 +1981,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
@@ -2029,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
@@ -2108,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.

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__