From 720306a1887011df5d08a70573bbb308fad94364 Mon Sep 17 00:00:00 2001 From: dborth Date: Sun, 20 Mar 2011 17:17:51 +0000 Subject: [PATCH] parsing fix, string safety --- source/filebrowser.cpp | 6 +++--- source/filebrowser.h | 2 +- source/fileop.cpp | 28 +++++++++++++++------------- source/freeze.cpp | 2 +- source/gcunzip.cpp | 2 +- source/gui/gui_keyboard.cpp | 6 ++---- source/menu.cpp | 23 +++++++++++------------ source/s9xsupport.cpp | 3 +-- 8 files changed, 35 insertions(+), 37 deletions(-) diff --git a/source/filebrowser.cpp b/source/filebrowser.cpp index 15da614..1a41af6 100644 --- a/source/filebrowser.cpp +++ b/source/filebrowser.cpp @@ -301,7 +301,7 @@ bool MakeFilePath(char filepath[], int type, char * filename, int filenum) sprintf (temppath, "%s%s/%s", pathPrefix[GCSettings.SaveMethod], folder, file); } CleanupPath(temppath); // cleanup path - strncpy(filepath, temppath, MAXPATHLEN); + snprintf(filepath, MAXPATHLEN, "%s", temppath); return true; } @@ -414,7 +414,7 @@ void StripExt(char* returnstring, char * inputstring) { char* loc_dot; - strncpy (returnstring, inputstring, MAXJOLIET); + snprintf (returnstring, MAXJOLIET, "%s", inputstring); if(inputstring == NULL || strlen(inputstring) < 4) return; @@ -549,8 +549,8 @@ int BrowserChangeFolder() if(!UpdateDirName()) return -1; - CleanupPath(browser.dir); HaltParseThread(); // halt parsing + CleanupPath(browser.dir); ResetBrowser(); // reset browser if(browser.dir[0] != 0) diff --git a/source/filebrowser.h b/source/filebrowser.h index 11b8315..4198fb7 100644 --- a/source/filebrowser.h +++ b/source/filebrowser.h @@ -26,7 +26,7 @@ typedef struct { - char dir[MAXPATHLEN]; // directory path of browserList + char dir[MAXPATHLEN + 1]; // directory path of browserList int numEntries; // # of entries in browserList int selIndex; // currently selected index of browserList int pageIndex; // starting index of browserList page display diff --git a/source/fileop.cpp b/source/fileop.cpp index b2eae0b..246153c 100644 --- a/source/fileop.cpp +++ b/source/fileop.cpp @@ -455,8 +455,8 @@ void CreateAppPath(char * origpath) path[2] = 'd'; } if(ChangeInterface(&path[pos], SILENT)) - strncpy(appPath, &path[pos], MAXPATHLEN); - appPath[MAXPATHLEN-1] = 0; + snprintf(appPath, MAXPATHLEN-1, "%s", &path[pos]); + free(path); } @@ -483,12 +483,12 @@ bool ParseDirEntries() char *ext; char path[MAXPATHLEN+1]; - struct dirent *entry; + struct dirent *entry = NULL; struct stat filestat; int i = 0; - while(i < 20) + while(i < 20 && !parseHalt) { entry = readdir(dir); @@ -529,7 +529,7 @@ bool ParseDirEntries() break; } - strncpy(browserList[browser.numEntries+i].filename, entry->d_name, MAXJOLIET); + snprintf(browserList[browser.numEntries+i].filename, MAXJOLIET, "%s", entry->d_name); browserList[browser.numEntries+i].length = filestat.st_size; browserList[browser.numEntries+i].mtime = filestat.st_mtime; browserList[browser.numEntries+i].isdir = (filestat.st_mode & _IFDIR) == 0 ? 0 : 1; // flag this as a dir @@ -539,7 +539,7 @@ bool ParseDirEntries() if(strcmp(entry->d_name, "..") == 0) sprintf(browserList[browser.numEntries+i].displayname, "Up One Level"); else - strncpy(browserList[browser.numEntries+i].displayname, browserList[browser.numEntries+i].filename, MAXJOLIET); + snprintf(browserList[browser.numEntries+i].displayname, MAXJOLIET, "%s", browserList[browser.numEntries+i].filename); browserList[browser.numEntries+i].icon = ICON_FOLDER; } else @@ -549,11 +549,14 @@ bool ParseDirEntries() i++; } - // Sort the file list - if(i >= 0) - qsort(browserList, browser.numEntries+i, sizeof(BROWSERENTRY), FileSortCallback); - - browser.numEntries += i; + if(!parseHalt) + { + // Sort the file list + if(i >= 0) + qsort(browserList, browser.numEntries+i, sizeof(BROWSERENTRY), FileSortCallback); + + browser.numEntries += i; + } if(entry == NULL || parseHalt) { @@ -561,7 +564,7 @@ bool ParseDirEntries() dir = NULL; // try to find and select the last loaded file - if(selectLoadedFile == 1 && parseHalt == 0 && loadedFile[0] != 0 && browser.dir[0] != 0) + if(selectLoadedFile == 1 && !parseHalt && loadedFile[0] != 0 && browser.dir[0] != 0) { int indexFound = -1; @@ -593,7 +596,6 @@ bool ParseDirEntries() } selectLoadedFile = 2; // selecting done } - return false; // no more entries } return true; // more entries diff --git a/source/freeze.cpp b/source/freeze.cpp index 109b1d5..2cd0d54 100644 --- a/source/freeze.cpp +++ b/source/freeze.cpp @@ -52,7 +52,7 @@ SaveSnapshot (char * filepath, bool silent) if(gameScreenPngSize > 0) { char screenpath[1024]; - strncpy(screenpath, filepath, 1024); + strcpy(screenpath, filepath); screenpath[strlen(screenpath)-4] = 0; sprintf(screenpath, "%s.png", screenpath); SaveFile((char *)gameScreenPng, screenpath, gameScreenPngSize, silent); diff --git a/source/gcunzip.cpp b/source/gcunzip.cpp index cd3bfb2..99f5ce8 100644 --- a/source/gcunzip.cpp +++ b/source/gcunzip.cpp @@ -457,7 +457,7 @@ int SzParse(char * filepath) } // parse information about this file to the file list structure - strncpy(browserList[SzJ].filename, SzF->Name, MAXJOLIET); + snprintf(browserList[SzJ].filename, MAXJOLIET, "%s", SzF->Name); StripExt(browserList[SzJ].displayname, browserList[SzJ].filename); browserList[SzJ].length = SzF->Size; // filesize browserList[SzJ].isdir = 0; // only files will be displayed (-> no flags) diff --git a/source/gui/gui_keyboard.cpp b/source/gui/gui_keyboard.cpp index 07a6562..db4a964 100644 --- a/source/gui/gui_keyboard.cpp +++ b/source/gui/gui_keyboard.cpp @@ -22,8 +22,7 @@ static char * GetDisplayText(char * t) if(len < MAX_KEYBOARD_DISPLAY) return t; - strncpy(tmptxt, &t[len-MAX_KEYBOARD_DISPLAY], MAX_KEYBOARD_DISPLAY); - tmptxt[MAX_KEYBOARD_DISPLAY-1] = 0; + snprintf(tmptxt, MAX_KEYBOARD_DISPLAY, "%s", &t[len-MAX_KEYBOARD_DISPLAY]); return &tmptxt[0]; } @@ -41,8 +40,7 @@ GuiKeyboard::GuiKeyboard(char * t, u32 max) focus = 0; // allow focus alignmentHor = ALIGN_CENTRE; alignmentVert = ALIGN_MIDDLE; - strncpy(kbtextstr, t, max); - kbtextstr[max] = 0; + snprintf(kbtextstr, 255, "%s", t); kbtextmaxlen = max; Key thekeys[4][11] = { diff --git a/source/menu.cpp b/source/menu.cpp index 9fa9227..ff52281 100644 --- a/source/menu.cpp +++ b/source/menu.cpp @@ -75,8 +75,8 @@ static lwp_t updatethread = LWP_THREAD_NULL; static bool guiHalt = true; static int showProgress = 0; -static char progressTitle[100]; -static char progressMsg[200]; +static char progressTitle[101]; +static char progressMsg[201]; static int progressDone = 0; static int progressTotal = 0; @@ -511,7 +511,7 @@ ShowProgress (const char *msg, int done, int total) if(showProgress != 1) CancelAction(); // wait for previous progress window to finish - strncpy(progressMsg, msg, 200); + snprintf(progressMsg, 200, "%s", msg); sprintf(progressTitle, "Please Wait"); showProgress = 1; progressTotal = total; @@ -534,7 +534,7 @@ ShowAction (const char *msg) if(showProgress != 0) CancelAction(); // wait for previous progress window to finish - strncpy(progressMsg, msg, 200); + snprintf(progressMsg, 200, "%s", msg); sprintf(progressTitle, "Please Wait"); showProgress = 2; progressDone = 0; @@ -1534,7 +1534,7 @@ static int MenuGameSaves(int action) SaveList saves; char filepath[1024]; char scrfile[1024]; - char tmp[MAXJOLIET]; + char tmp[MAXJOLIET+1]; struct stat filestat; struct tm * timeinfo; int device = GCSettings.SaveMethod; @@ -1627,7 +1627,7 @@ static int MenuGameSaves(int action) else continue; - strncpy(tmp, browserList[i].filename, MAXJOLIET); + strcpy(tmp, browserList[i].filename); tmp[len2-4] = 0; n = FindGameSaveNum(tmp, device); @@ -1635,7 +1635,7 @@ static int MenuGameSaves(int action) { saves.type[j] = type; saves.files[saves.type[j]][n] = 1; - strncpy(saves.filename[j], browserList[i].filename, MAXJOLIET); + strcpy(saves.filename[j], browserList[i].filename); if(saves.type[j] == FILE_SNAPSHOT) { @@ -3729,11 +3729,10 @@ static int MenuSettingsNetwork() if(ret >= 0 || firstRun) { firstRun = false; - strncpy (options.value[0], GCSettings.smbip, 25); - options.value[0][25] = 0; - strncpy (options.value[1], GCSettings.smbshare, 19); - strncpy (options.value[2], GCSettings.smbuser, 19); - strncpy (options.value[3], GCSettings.smbpwd, 19); + snprintf (options.value[0], 25, "%s", GCSettings.smbip); + snprintf (options.value[1], 19, "%s", GCSettings.smbshare); + snprintf (options.value[2], 19, "%s", GCSettings.smbuser); + snprintf (options.value[3], 19, "%s", GCSettings.smbpwd); optionBrowser.TriggerUpdate(); } diff --git a/source/s9xsupport.cpp b/source/s9xsupport.cpp index 9749ca7..579c9f3 100644 --- a/source/s9xsupport.cpp +++ b/source/s9xsupport.cpp @@ -42,8 +42,7 @@ void S9xExit() void S9xMessage(int /*type */, int /*number */, const char *message) { static char buffer[MAX_MESSAGE_LEN + 1]; - strncpy(buffer, message, MAX_MESSAGE_LEN); - buffer[MAX_MESSAGE_LEN] = 0; + snprintf(buffer, MAX_MESSAGE_LEN, "%s", message); S9xSetInfoString(buffer); }