better buffer size safety with COM_StripExtension, COM_FileBase and COM_DefaultExtension

git-svn-id: http://svn.code.sf.net/p/quakespasm/code/trunk/quakespasm@554 af15c1b1-3010-417e-b628-4374ebc0bcbd

Quake/cl_demo.c

@@ -251,7 +251,7 @@ void CL_Record_f (void)
 //
 // open the demo file
 //
-	COM_DefaultExtension (name, ".dem");
+	COM_DefaultExtension (name, ".dem", sizeof(name));
 
 	Con_Printf ("recording to %s.\n", name);
 	cls.demofile = fopen (name, "wb");
@@ -302,7 +302,7 @@ void CL_PlayDemo_f (void)
 // open the demo file
 //
 	strcpy (name, Cmd_Argv(1));
-	COM_DefaultExtension (name, ".dem");
+	COM_DefaultExtension (name, ".dem", sizeof(name));
 
 	Con_Printf ("Playing demo from %s.\n", name);
 

Quake/cl_parse.c

@@ -350,7 +350,7 @@ void CL_ParseServerInfo (void)
 //
 
 	// copy the naked name of the map file to the cl structure -- O.S
-	COM_StripExtension (COM_SkipPath(model_precache[1]), cl.mapname);
+	COM_StripExtension (COM_SkipPath(model_precache[1]), cl.mapname, sizeof(cl.mapname));
 
 	for (i=1 ; i<nummodels ; i++)
 	{

Quake/common.c

@@ -893,13 +893,13 @@ COM_SkipPath
 */
 const char *COM_SkipPath (const char *pathname)
 {
-	const char *last;
+	const char	*last;
 
 	last = pathname;
 	while (*pathname)
 	{
-		if (*pathname=='/')
-			last = pathname+1;
+		if (*pathname == '/')
+			last = pathname + 1;
 		pathname++;
 	}
 	return last;
@@ -910,11 +910,29 @@ const char *COM_SkipPath (const char *pathname)
 COM_StripExtension
 ============
 */
-void COM_StripExtension (const char *in, char *out)
+void COM_StripExtension (const char *in, char *out, size_t outsize)
 {
-	while (*in && *in != '.')
-		*out++ = *in++;
-	*out = 0;
+	int	length;
+
+	if (!*in)
+	{
+		*out = '\0';
+		return;
+	}
+	if (in != out)	/* copy when not in-place editing */
+	{
+		strncpy (out, in, outsize - 1);
+		out[outsize - 1] = '\0';
+	}
+	length = (int)strlen(out) - 1;
+	while (length > 0 && out[length] != '.')
+	{
+		--length;
+		if (out[length] == '/' || out[length] == '\\')
+			return;	/* no extension */
+	}
+	if (length > 0)
+		out[length] = '\0';
 }
 
 /*
@@ -943,52 +961,74 @@ const char *COM_FileGetExtension (const char *in)
 /*
 ============
 COM_FileBase
+take 'somedir/otherdir/filename.ext',
+write only 'filename' to the output
 ============
 */
-void COM_FileBase (const char *in, char *out)
+void COM_FileBase (const char *in, char *out, size_t outsize)
 {
-	const char *s, *s2;
+	const char	*dot, *slash, *s;
 
-	s = in + strlen(in) - 1;
+	s = in;
+	slash = in;
+	dot = NULL;
+	while (*s)
+	{
+		if (*s == '/')
+			slash = s + 1;
+		if (*s == '.')
+			dot = s;
+		s++;
+	}
+	if (dot == NULL)
+		dot = s;
 
-	while (s != in && *s != '.')
-		s--;
-
-	for (s2 = s ; s2 != in && *s2 && *s2 != '/' ; s2--)
-	;
-
-	if (s-s2 < 2)
-		strcpy (out,"?model?");
+	if (dot - slash < 2)
+	{
+		size_t	len = outsize - 1;
+		strncpy (out, "?model?", len);
+		out[len] = '\0';
+	}
 	else
 	{
-		s--;
-		strncpy (out,s2+1, s-s2);
-		out[s-s2] = 0;
+		size_t	len = dot - slash;
+		if (len >= outsize)
+			len = outsize - 1;
+		memcpy (out, slash, len);
+		out[len] = '\0';
 	}
 }
 
-
 /*
 ==================
 COM_DefaultExtension
 ==================
 */
-void COM_DefaultExtension (char *path, const char *extension)
+void COM_DefaultExtension (char *path, const char *extension, size_t len)
 {
-	char    *src;
+	char	*src;
+	size_t	l;
 //
 // if path doesn't have a .EXT, append extension
 // (extension should include the .)
 //
-	src = path + strlen(path) - 1;
+	if (!*path)
+		return;
+	l = strlen(path);
+	src = path + l - 1;
 
 	while (*src != '/' && src != path)
 	{
 		if (*src == '.')
-			return;                 // it has an extension
+			return;		// it has an extension
 		src--;
 	}
 
+	if (l + strlen(extension) >= len)	// buf overrun
+	{
+	//	Sys_Error("bufsize too small");
+		return;
+	}
 	strcat (path, extension);
 }
 
@@ -1663,7 +1703,7 @@ byte *COM_LoadFile (const char *path, int usehunk, unsigned int *path_id)
 		return NULL;
 
 // extract the filename base name for hunk tag
-	COM_FileBase (path, base);
+	COM_FileBase (path, base, sizeof(base));
 
 	switch (usehunk)
 	{

Quake/common.h

@@ -175,9 +175,9 @@ void COM_Init (void);
 void COM_InitArgv (int argc, char **argv);
 
 const char *COM_SkipPath (const char *pathname);
-void COM_StripExtension (const char *in, char *out);
-void COM_FileBase (const char *in, char *out);
-void COM_DefaultExtension (char *path, const char *extension);
+void COM_StripExtension (const char *in, char *out, size_t outsize);
+void COM_FileBase (const char *in, char *out, size_t outsize);
+void COM_DefaultExtension (char *path, const char *extension, size_t len);
 const char *COM_FileGetExtension (const char *in);
 void COM_CreatePath (char *path);
 

Quake/console.c

@@ -178,7 +178,7 @@ void Con_Dump_f (void)
 			return;
 		}
 		sprintf (name, "%s/%s", com_gamedir, Cmd_Argv(1));
-		COM_DefaultExtension (name, ".txt");
+		COM_DefaultExtension (name, ".txt", sizeof(name));
 	}
 	else
 		sprintf (name, "%s/condump.txt", com_gamedir);

Quake/gl_model.c

@@ -292,7 +292,7 @@ model_t *Mod_LoadModel (model_t *mod, qboolean crash)
 //
 // allocate a new model
 //
-	COM_FileBase (mod->name, loadname);
+	COM_FileBase (mod->name, loadname, sizeof(loadname));
 
 	loadmodel = mod;
 
@@ -442,7 +442,7 @@ void Mod_LoadTextures (lump_t *l)
 			{
 				//external textures -- first look in "textures/mapname/" then look in "textures/"
 				mark = Hunk_LowMark();
-				COM_StripExtension (loadmodel->name + 5, mapname);
+				COM_StripExtension (loadmodel->name + 5, mapname, sizeof(mapname));
 				sprintf (filename, "textures/%s/#%s", mapname, tx->name+1); //this also replaces the '*' with a '#'
 				data = Image_LoadImage (filename, &fwidth, &fheight);
 				if (!data)
@@ -478,7 +478,7 @@ void Mod_LoadTextures (lump_t *l)
 			{
 				//external textures -- first look in "textures/mapname/" then look in "textures/"
 				mark = Hunk_LowMark ();
-				COM_StripExtension (loadmodel->name + 5, mapname);
+				COM_StripExtension (loadmodel->name + 5, mapname, sizeof(mapname));
 				sprintf (filename, "textures/%s/%s", mapname, tx->name);
 				data = Image_LoadImage (filename, &fwidth, &fheight);
 				if (!data)
@@ -637,13 +637,13 @@ void Mod_LoadLighting (lump_t *l)
 	int i, mark;
 	byte *in, *out, *data;
 	byte d;
-	char litfilename[1024];
+	char litfilename[MAX_OSPATH];
 	unsigned int path_id;
 
 	loadmodel->lightdata = NULL;
 	// LordHavoc: check for a .lit file
 	strcpy(litfilename, loadmodel->name);
-	COM_StripExtension(litfilename, litfilename);
+	COM_StripExtension(litfilename, litfilename, sizeof(litfilename));
 	strcat(litfilename, ".lit");
 	mark = Hunk_LowMark();
 	data = (byte*) COM_LoadHunkFile (litfilename, &path_id);
@@ -728,7 +728,7 @@ void Mod_LoadEntities (lump_t *l)
 		goto _load_embedded;
 
 	strcpy(entfilename, loadmodel->name);
-	COM_StripExtension(entfilename, entfilename);
+	COM_StripExtension(entfilename, entfilename, sizeof(entfilename));
 	strcat(entfilename, ".ent");
 	Con_DPrintf("trying to load %s\n", entfilename);
 	mark = Hunk_LowMark();

Quake/host_cmd.c

@@ -292,7 +292,7 @@ void ExtraMaps_Init (void)
 			{
 				if (!strstr(dir_t->d_name, ".bsp") && !strstr(dir_t->d_name, ".BSP"))
 					continue;
-				COM_StripExtension(dir_t->d_name, mapname);
+				COM_StripExtension(dir_t->d_name, mapname, sizeof(mapname));
 				ExtraMaps_Add (mapname);
 			}
 			closedir(dir_p);
@@ -307,7 +307,7 @@ void ExtraMaps_Init (void)
 					{
 						if (pak->files[i].filelen > 32*1024)
 						{ // don't list files under 32k (ammo boxes etc)
-							COM_StripExtension(pak->files[i].name + 5, mapname);
+							COM_StripExtension(pak->files[i].name + 5, mapname, sizeof(mapname));
 							ExtraMaps_Add (mapname);
 						}
 					}
@@ -1000,7 +1000,7 @@ Host_Savegame_f
 */
 void Host_Savegame_f (void)
 {
-	char	name[256];
+	char	name[MAX_OSPATH];
 	FILE	*f;
 	int	i;
 	char	comment[SAVEGAME_COMMENT_LENGTH+1];
@@ -1048,7 +1048,7 @@ void Host_Savegame_f (void)
 	}
 
 	sprintf (name, "%s/%s", com_gamedir, Cmd_Argv(1));
-	COM_DefaultExtension (name, ".sav");
+	COM_DefaultExtension (name, ".sav", sizeof(name));
 
 	Con_Printf ("Saving game to %s...\n", name);
 	f = fopen (name, "w");
@@ -1120,7 +1120,7 @@ void Host_Loadgame_f (void)
 	cls.demonum = -1;		// stop demo loop in case this fails
 
 	sprintf (name, "%s/%s", com_gamedir, Cmd_Argv(1));
-	COM_DefaultExtension (name, ".sav");
+	COM_DefaultExtension (name, ".sav", sizeof(name));
 
 // we can't call SCR_BeginLoadingPlaque, because too much stack space has
 // been used.  The menu calls it before stuffing loadgame command