From eb3a4033c7d6f1d5e042edd5f416bbc257e40975 Mon Sep 17 00:00:00 2001 From: Simon Howard Date: Sat, 29 Mar 2014 21:49:57 -0400 Subject: strife: Eliminate use of unsafe string functions. Eliminate use of strcpy, strcat, strncpy, and use the new safe alternatives. --- src/strife/d_main.c | 27 ++++++++++++++++----------- src/strife/d_net.c | 5 +++-- src/strife/g_game.c | 41 +++++++++++++++++++++++------------------ src/strife/hu_stuff.c | 9 ++++++--- src/strife/m_menu.c | 37 ++++++++++++++++++++++++------------- src/strife/m_saves.c | 2 +- src/strife/p_dialog.c | 10 +++++----- src/strife/p_dialog.h | 6 ++++-- src/strife/p_enemy.c | 3 ++- src/strife/p_inter.c | 1 + src/strife/p_switch.c | 1 + src/strife/r_data.c | 12 ++++++------ src/strife/s_sound.c | 6 +++--- src/strife/wi_stuff.c | 4 ++-- 14 files changed, 97 insertions(+), 67 deletions(-) diff --git a/src/strife/d_main.c b/src/strife/d_main.c index 6769c8ed..edd01f87 100644 --- a/src/strife/d_main.c +++ b/src/strife/d_main.c @@ -783,20 +783,26 @@ static char *GetGameName(char *gamename) if (deh_sub != banners[i]) { + size_t gamename_size; + // Has been replaced // We need to expand via printf to include the Doom version // number // We also need to cut off spaces to get the basic name - gamename = Z_Malloc(strlen(deh_sub) + 10, PU_STATIC, 0); - sprintf(gamename, deh_sub, STRIFE_VERSION / 100, STRIFE_VERSION % 100); + gamename_size = strlen(deh_sub) + 10; + gamename = Z_Malloc(gamename_size, PU_STATIC, 0); + snprintf(gamename, gamename_size, deh_sub, + STRIFE_VERSION / 100, STRIFE_VERSION % 100); while (gamename[0] != '\0' && isspace(gamename[0])) - strcpy(gamename, gamename+1); + { + memmove(gamename, gamename + 1, gamename_size - 1); + } while (gamename[0] != '\0' && isspace(gamename[strlen(gamename)-1])) gamename[strlen(gamename) - 1] = '\0'; - + return gamename; } } @@ -848,7 +854,7 @@ void D_IdentifyVersion(void) // how the heck is Choco surviving without this routine? sepchar = M_GetFilePath(iwad, filename, len); filename[strlen(filename)] = sepchar; - strcat(filename, "voices.wad"); + M_StringConcat(filename, "voices.wad", sizeof(filename)); if(!M_FileExists(filename)) disable_voices = 1; @@ -1602,17 +1608,17 @@ void D_DoomMain (void) { if (!strcasecmp(myargv[p+1] + strlen(myargv[p+1]) - 4, ".lmp")) { - strcpy(file, myargv[p + 1]); + M_StringCopy(file, myargv[p + 1], sizeof(file)); } else { - sprintf (file,"%s.lmp", myargv[p+1]); + snprintf(file, sizeof(file), "%s.lmp", myargv[p+1]); } if (D_AddFile (file)) { - strncpy(demolumpname, lumpinfo[numlumps - 1].name, 8); - demolumpname[8] = '\0'; + M_StringCopy(demolumpname, lumpinfo[numlumps - 1].name, + sizeof(demolumpname)); printf("Playing demo %s.\n", file); } @@ -1622,8 +1628,7 @@ void D_DoomMain (void) // the demo in the same way as Vanilla Doom. This makes // tricks like "-playdemo demo1" possible. - strncpy(demolumpname, myargv[p + 1], 8); - demolumpname[8] = '\0'; + M_StringCopy(demolumpname, myargv[p + 1], sizeof(demolumpname)); } } diff --git a/src/strife/d_net.c b/src/strife/d_net.c index 065f74e1..b4f125d2 100644 --- a/src/strife/d_net.c +++ b/src/strife/d_net.c @@ -32,6 +32,7 @@ #include "d_main.h" #include "m_argv.h" #include "m_menu.h" +#include "m_misc.h" #include "i_system.h" #include "i_timer.h" #include "i_video.h" @@ -58,8 +59,8 @@ static void PlayerQuitGame(player_t *player) // Do this the same way as Vanilla Doom does, to allow dehacked // replacements of this message - strncpy(exitmsg, DEH_String("Player 1 left the game"), sizeof(exitmsg)); - exitmsg[sizeof(exitmsg) - 1] = '\0'; + M_StringCopy(exitmsg, DEH_String("Player 1 left the game"), + sizeof(exitmsg)); exitmsg[7] += player_num; diff --git a/src/strife/g_game.c b/src/strife/g_game.c index 5c3bfe03..8bb068fe 100644 --- a/src/strife/g_game.c +++ b/src/strife/g_game.c @@ -39,6 +39,7 @@ #include "m_controls.h" #include "m_misc.h" #include "m_menu.h" +#include "m_misc.h" #include "m_saves.h" // STRIFE #include "m_random.h" #include "i_system.h" @@ -1024,7 +1025,10 @@ void G_Ticker (void) case BTS_SAVEGAME: if (!character_name[0]) // [STRIFE] - strcpy (character_name, "NET GAME"); + { + M_StringCopy(character_name, "NET GAME", + sizeof(character_name)); + } savegameslot = (players[i].cmd.buttons & BTS_SAVEMASK)>>BTS_SAVESHIFT; gameaction = ga_savegame; @@ -1172,7 +1176,8 @@ void G_PlayerReborn (int player) p->inventory[i].type = NUMMOBJTYPES; // villsa [STRIFE]: Default objective - strncpy(mission_objective, DEH_String("Find help"), OBJECTIVE_LEN); + M_StringCopy(mission_objective, DEH_String("Find help"), + OBJECTIVE_LEN); } // @@ -1656,13 +1661,13 @@ char savename[256]; // [STRIFE]: No such function. /* -void G_LoadGame (char* name) -{ - strcpy (savename, name); - gameaction = ga_loadgame; -} +void G_LoadGame (char* name) +{ + M_StringCopy(savename, name, sizeof(savename)); + gameaction = ga_loadgame; +} */ - + // haleyjd 20100928: [STRIFE] VERSIONSIZE == 8 #define VERSIONSIZE 8 @@ -1755,7 +1760,7 @@ boolean G_WriteSaveName(int slot, const char *charname) // haleyjd: memset full character_name for safety memset(character_name, 0, CHARACTER_NAME_LEN); - strcpy(character_name, charname); + M_StringCopy(character_name, charname, sizeof(character_name)); // haleyjd: use M_SafeFilePath tmpname = M_SafeFilePath(savepathtemp, "name"); @@ -1771,7 +1776,7 @@ boolean G_WriteSaveName(int slot, const char *charname) // // G_SaveGame // Called by the menu task. -// Description is a 24 byte text string +// Description is a 24 byte text string // // [STRIFE] No such function, at least in v1.2 // STRIFE-TODO: Does this make a comeback in v1.31? @@ -1779,14 +1784,14 @@ boolean G_WriteSaveName(int slot, const char *charname) void G_SaveGame ( int slot, - char* description ) -{ - savegameslot = slot; - strcpy (savedescription, description); - sendsave = true; -} + char* description ) +{ + savegameslot = slot; + M_StringCopy(savedescription, description, sizeof(savedescription)); + sendsave = true; +} */ - + void G_DoSaveGame (char *path) { char *current_path; @@ -1859,7 +1864,7 @@ void G_DoSaveGame (char *path) Z_Free(savegame_file); gameaction = ga_nothing; - //strcpy(savedescription, ""); + //M_StringCopy(savedescription, "", sizeof(savedescription)); // [STRIFE]: custom message logic if(!strcmp(path, savepath)) diff --git a/src/strife/hu_stuff.c b/src/strife/hu_stuff.c index b9e13f8a..85ccb44f 100644 --- a/src/strife/hu_stuff.c +++ b/src/strife/hu_stuff.c @@ -38,6 +38,7 @@ #include "hu_stuff.h" #include "hu_lib.h" #include "m_controls.h" +#include "m_misc.h" #include "w_wad.h" #include "s_sound.h" @@ -599,7 +600,7 @@ boolean HU_Responder(event_t *ev) // leave chat mode and notify that it was sent chat_on = false; - strcpy(lastmessage, chat_macros[c]); + M_StringCopy(lastmessage, chat_macros[c], sizeof(lastmessage)); plr->message = lastmessage; eatkey = true; } @@ -650,7 +651,8 @@ boolean HU_Responder(event_t *ev) { eatkey = true; HU_queueChatChar(HU_CHANGENAME); - strncpy(lastmessage, DEH_String("Changing Name:"), sizeof(lastmessage)); + M_StringCopy(lastmessage, DEH_String("Changing Name:"), + sizeof(lastmessage)); plr->message = lastmessage; hu_setting_name = true; } @@ -677,7 +679,8 @@ boolean HU_Responder(event_t *ev) } else { - strcpy(lastmessage, w_chat.l.l); + M_StringCopy(lastmessage, w_chat.l.l, + sizeof(lastmessage)); } plr->message = lastmessage; } diff --git a/src/strife/m_menu.c b/src/strife/m_menu.c index 27a9045c..7f4f18fc 100644 --- a/src/strife/m_menu.c +++ b/src/strife/m_menu.c @@ -54,6 +54,7 @@ #include "m_argv.h" #include "m_controls.h" +#include "m_misc.h" #include "m_saves.h" // [STRIFE] #include "p_saveg.h" @@ -560,7 +561,8 @@ void M_ReadSaveStrings(void) handle = fopen(fname, "rb"); if(handle == NULL) { - strcpy(savegamestrings[i], EMPTYSTRING); + M_StringCopy(savegamestrings[i], EMPTYSTRING, + sizeof(savegamestrings[i])); LoadMenu[i].status = 0; continue; } @@ -765,7 +767,7 @@ void M_SaveSelect(int choice) quickSaveSlot = choice; //saveSlot = choice; - strcpy(saveOldString,savegamestrings[choice]); + M_StringCopy(saveOldString, savegamestrings[choice], sizeof(saveOldString)); if (!strcmp(savegamestrings[choice],EMPTYSTRING)) savegamestrings[choice][0] = 0; saveCharIndex = strlen(savegamestrings[choice]); @@ -1850,7 +1852,8 @@ boolean M_Responder (event_t* ev) case KEY_ESCAPE: saveStringEnter = 0; - strcpy(&savegamestrings[quickSaveSlot][0],saveOldString); + M_StringCopy(savegamestrings[quickSaveSlot], saveOldString, + sizeof(savegamestrings[quickSaveSlot])); break; case KEY_ENTER: @@ -2286,24 +2289,32 @@ void M_Drawer (void) int foundnewline = 0; for (i = 0; i < strlen(messageString + start); i++) + { if (messageString[start + i] == '\n') { - memset(string, 0, sizeof(string)); - strncpy(string, messageString + start, i); + M_StringCopy(string, messageString + start, + sizeof(string)); + if (i < sizeof(string)) + { + string[i] = '\0'; + } + foundnewline = 1; start += i + 1; break; } + } - if (!foundnewline) - { - strcpy(string, messageString + start); - start += strlen(string); - } + if (!foundnewline) + { + M_StringCopy(string, messageString + start, + sizeof(string)); + start += strlen(string); + } - x = 160 - M_StringWidth(string) / 2; - M_WriteText(x, y, string); - y += SHORT(hu_font[0]->height); + x = 160 - M_StringWidth(string) / 2; + M_WriteText(x, y, string); + y += SHORT(hu_font[0]->height); } return; diff --git a/src/strife/m_saves.c b/src/strife/m_saves.c index a4068c4f..2908b694 100644 --- a/src/strife/m_saves.c +++ b/src/strife/m_saves.c @@ -501,7 +501,7 @@ char M_GetFilePath(const char *fn, char *dest, size_t len) p = dest + len - 1; - strncpy(dest, fn, len); + M_StringCopy(dest, fn, len); while(p >= dest) { diff --git a/src/strife/p_dialog.c b/src/strife/p_dialog.c index 070288aa..95723a56 100644 --- a/src/strife/p_dialog.c +++ b/src/strife/p_dialog.c @@ -38,6 +38,7 @@ #include "doomstat.h" #include "m_random.h" #include "m_menu.h" +#include "m_misc.h" #include "r_main.h" #include "v_video.h" #include "p_local.h" @@ -719,7 +720,7 @@ boolean P_GiveItemToPlayer(player_t *player, int sprnum, mobjtype_t type) { if(mobjinfo[type].name) { - strncpy(pickupstring, DEH_String(mobjinfo[type].name), 39); + M_StringCopy(pickupstring, DEH_String(mobjinfo[type].name), 39); player->message = pickupstring; } player->questflags |= 1 << (type - MT_TOKEN_QUEST1); @@ -1132,10 +1133,9 @@ static void P_DialogDrawer(void) if(currentdialog->choices[i].needamounts[0] > 0) { // haleyjd 20120401: necessary to avoid undefined behavior: - strcpy(choicetext2, choicetext); + M_StringCopy(choicetext2, choicetext, sizeof(choicetext2)); DEH_snprintf(choicetext, sizeof(choicetext), - "%s for %d", - choicetext2, + "%s for %d", choicetext2, currentdialog->choices[i].needamounts[0]); } @@ -1228,7 +1228,7 @@ void P_DialogDoChoice(int choice) { DEH_snprintf(mission_objective, OBJECTIVE_LEN, "log%i", objective); objlump = W_CacheLumpName(mission_objective, PU_CACHE); - strncpy(mission_objective, objlump, OBJECTIVE_LEN); + M_StringCopy(mission_objective, objlump, OBJECTIVE_LEN); } // haleyjd 20130301: v1.31 hack: if first char of message is a period, // clear the player's message. Is this actually used anywhere? diff --git a/src/strife/p_dialog.h b/src/strife/p_dialog.h index 7463490b..44b1da6a 100644 --- a/src/strife/p_dialog.h +++ b/src/strife/p_dialog.h @@ -52,7 +52,8 @@ extern int dialogshowtext; do { \ int obj_ln = W_CheckNumForName(DEH_String(x)); \ if(obj_ln > minlumpnum) \ - strncpy(mission_objective, W_CacheLumpNum(obj_ln, PU_CACHE), OBJECTIVE_LEN);\ + M_StringCopy(mission_objective, W_CacheLumpNum(obj_ln, PU_CACHE), \ + OBJECTIVE_LEN);\ } while(0) // haleyjd - voice and objective in one @@ -61,7 +62,8 @@ do { \ int obj_ln = W_CheckNumForName(DEH_String(log)); \ I_StartVoice(DEH_String(voice)); \ if(obj_ln > minlumpnum) \ - strncpy(mission_objective, W_CacheLumpNum(obj_ln, PU_CACHE), OBJECTIVE_LEN);\ + M_StringCopy(mission_objective, W_CacheLumpNum(obj_ln, PU_CACHE), \ + OBJECTIVE_LEN);\ } while(0) typedef struct mapdlgchoice_s diff --git a/src/strife/p_enemy.c b/src/strife/p_enemy.c index 684d6b27..c3944331 100644 --- a/src/strife/p_enemy.c +++ b/src/strife/p_enemy.c @@ -32,6 +32,7 @@ #include "m_random.h" #include "i_system.h" #include "doomdef.h" +#include "m_misc.h" #include "p_local.h" #include "s_sound.h" #include "g_game.h" @@ -2706,7 +2707,7 @@ void A_QuestMsg(mobj_t* actor) // get name name = DEH_String(mobjinfo[(MT_TOKEN_QUEST1 - 1) + actor->info->speed].name); - strcpy(pmsgbuffer, name); // inlined in asm + M_StringCopy(pmsgbuffer, name, sizeof(pmsgbuffer)); // inlined in asm // give quest and display message to players for(i = 0; i < MAXPLAYERS; i++) diff --git a/src/strife/p_inter.c b/src/strife/p_inter.c index a0166469..573a5efd 100644 --- a/src/strife/p_inter.c +++ b/src/strife/p_inter.c @@ -31,6 +31,7 @@ #include "deh_main.h" #include "deh_misc.h" #include "doomstat.h" +#include "m_misc.h" #include "m_random.h" #include "i_system.h" #include "am_map.h" diff --git a/src/strife/p_switch.c b/src/strife/p_switch.c index bb70f890..d9d28192 100644 --- a/src/strife/p_switch.c +++ b/src/strife/p_switch.c @@ -41,6 +41,7 @@ #include "p_dialog.h" #include "p_local.h" // haleyjd [STRIFE] #include "m_bbox.h" // villsa [STRIFE] +#include "m_misc.h" // Data. #include "sounds.h" diff --git a/src/strife/r_data.c b/src/strife/r_data.c index b8fa5686..51e8da67 100644 --- a/src/strife/r_data.c +++ b/src/strife/r_data.c @@ -37,6 +37,7 @@ #include "r_local.h" #include "p_local.h" #include "doomstat.h" +#include "m_misc.h" #include "r_sky.h" #include "r_data.h" #include "sounds.h" // villsa [STRIFE] @@ -485,21 +486,20 @@ void R_InitTextures (void) int temp2; int temp3; - + // Load the patch names from pnames.lmp. - name[8] = 0; names = W_CacheLumpName (DEH_String("PNAMES"), PU_STATIC); nummappatches = LONG ( *((int *)names) ); name_p = names+4; patchlookup = Z_Malloc(nummappatches*sizeof(*patchlookup), PU_STATIC, NULL); - - for (i=0 ; isfx.name, name, 8); + M_StringCopy(voice->sfx.name, name, sizeof(voice->sfx.name)); voice->sfx.priority = INT_MIN; // make highest possible priority voice->sfx.pitch = -1; voice->sfx.volume = -1; @@ -607,8 +608,7 @@ void I_StartVoice(const char *lumpname) return; // Because of constness problems... - strncpy(lumpnamedup, lumpname, 9); - lumpnamedup[8] = '\0'; + M_StringCopy(lumpnamedup, lumpname, sizeof(lumpnamedup)); if((lumpnum = W_CheckNumForName(lumpnamedup)) != -1) { diff --git a/src/strife/wi_stuff.c b/src/strife/wi_stuff.c index f322e31f..8521db9f 100644 --- a/src/strife/wi_stuff.c +++ b/src/strife/wi_stuff.c @@ -1697,12 +1697,12 @@ static void WI_loadUnloadData(load_callback_t callback) if (gamemode == commercial) { - strncpy(name, DEH_String("INTERPIC"), 9); + M_StringCopy(name, DEH_String("INTERPIC"), sizeof(name)); name[8] = '\0'; } else if (gamemode == retail && wbs->epsd == 3) { - strncpy(name, DEH_String("INTERPIC"), 9); + M_StringCopy(name, DEH_String("INTERPIC"), sizeof(name)); name[8] = '\0'; } else -- cgit v1.2.3