From 8494877f84eadbb3119fe30a5d023416d8c445d4 Mon Sep 17 00:00:00 2001 From: sezero Date: Tue, 27 Dec 2011 08:04:02 +0000 Subject: [PATCH] better buffer size safety with COM_StripExtension, COM_FileBase and COM_DefaultExtension git-svn-id: svn+ssh://svn.code.sf.net/p/quakespasm/code/trunk@554 af15c1b1-3010-417e-b628-4374ebc0bcbd --- quakespasm/Quake/cl_demo.c | 4 +- quakespasm/Quake/cl_parse.c | 2 +- quakespasm/Quake/common.c | 94 ++++++++++++++++++++++++++----------- quakespasm/Quake/common.h | 6 +-- quakespasm/Quake/console.c | 2 +- quakespasm/Quake/gl_model.c | 12 ++--- quakespasm/Quake/host_cmd.c | 10 ++-- 7 files changed, 85 insertions(+), 45 deletions(-) diff --git a/quakespasm/Quake/cl_demo.c b/quakespasm/Quake/cl_demo.c index 077f4ff4..8b306262 100644 --- a/quakespasm/Quake/cl_demo.c +++ b/quakespasm/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); diff --git a/quakespasm/Quake/cl_parse.c b/quakespasm/Quake/cl_parse.c index 151269cd..de89f683 100644 --- a/quakespasm/Quake/cl_parse.c +++ b/quakespasm/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 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) { diff --git a/quakespasm/Quake/common.h b/quakespasm/Quake/common.h index 2c1a867d..6bfeb457 100644 --- a/quakespasm/Quake/common.h +++ b/quakespasm/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); diff --git a/quakespasm/Quake/console.c b/quakespasm/Quake/console.c index 5b5d846f..d1f220a1 100644 --- a/quakespasm/Quake/console.c +++ b/quakespasm/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); diff --git a/quakespasm/Quake/gl_model.c b/quakespasm/Quake/gl_model.c index 049f75d6..c6b4906e 100644 --- a/quakespasm/Quake/gl_model.c +++ b/quakespasm/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(); diff --git a/quakespasm/Quake/host_cmd.c b/quakespasm/Quake/host_cmd.c index 87c2f5c5..d58ccc18 100644 --- a/quakespasm/Quake/host_cmd.c +++ b/quakespasm/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