From aad52488acf93c2deebd772182b329171c08ce39 Mon Sep 17 00:00:00 2001 From: harry Date: Sat, 21 Jan 2023 09:59:50 -0500 Subject: [PATCH] Bug fix for ASAN error overlapping strcpy source and destination caused curMovieFilename being passed to openRecordingMovie which copies over itself. Changed type from char array to std::string whose assign function will allocate new memory to copy to before deleting the old string buffer. --- src/drivers/Qt/AviRecord.cpp | 2 +- src/drivers/win/taseditor.cpp | 4 ++-- src/drivers/win/window.cpp | 2 +- src/file.cpp | 9 ++++----- src/movie.cpp | 20 ++++++++++---------- src/movie.h | 2 +- 6 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/drivers/Qt/AviRecord.cpp b/src/drivers/Qt/AviRecord.cpp index cd940915..7a22e263 100644 --- a/src/drivers/Qt/AviRecord.cpp +++ b/src/drivers/Qt/AviRecord.cpp @@ -853,7 +853,7 @@ static void log_callback( void *avcl, int level, const char *fmt, va_list vl) int loadCodecConfig( int type, const char *codec_name, AVCodecContext *ctx) { int i,j; - char filename[512]; + char filename[4096]; char line[512]; char section[256], id[256], val[256]; void *obj, *child; diff --git a/src/drivers/win/taseditor.cpp b/src/drivers/win/taseditor.cpp index b643abe9..3b52f200 100644 --- a/src/drivers/win/taseditor.cpp +++ b/src/drivers/win/taseditor.cpp @@ -236,8 +236,8 @@ void updateTASEditor() TaseditorAutoFunction(); // but we still should run Lua auto function if (mustEngageTaseditor) { - char fullname[1000]; - strcpy(fullname, curMovieFilename); + char fullname[2048]; + strcpy(fullname, curMovieFilename.c_str()); if (enterTASEditor()) loadProject(fullname); mustEngageTaseditor = false; diff --git a/src/drivers/win/window.cpp b/src/drivers/win/window.cpp index a781b75e..5c3382f7 100644 --- a/src/drivers/win/window.cpp +++ b/src/drivers/win/window.cpp @@ -3216,7 +3216,7 @@ void SaveMovieAs() ofn.hInstance=fceu_hInstance; ofn.lpstrTitle="Save Movie as..."; ofn.lpstrFilter=filter; - strcpy(nameo,curMovieFilename); + strcpy(nameo,curMovieFilename.c_str()); ofn.lpstrFile=nameo; ofn.lpstrDefExt="fm2"; ofn.nMaxFile=256; diff --git a/src/file.cpp b/src/file.cpp index e4d5b4f6..017495a3 100644 --- a/src/file.cpp +++ b/src/file.cpp @@ -446,13 +446,12 @@ int FCEU_fisarchive(FCEUFILE *fp) std::string GetMfn() //Retrieves the movie filename from curMovieFilename (for adding to savestate and auto-save files) { std::string movieFilenamePart; - extern char curMovieFilename[512]; - if(*curMovieFilename) - { + if (!curMovieFilename.empty()) + { char drv[PATH_MAX], dir[PATH_MAX], name[PATH_MAX], ext[PATH_MAX]; - splitpath(curMovieFilename,drv,dir,name,ext); + splitpath(curMovieFilename.c_str(),drv,dir,name,ext); movieFilenamePart = std::string(".") + name; - } + } return movieFilenamePart; } diff --git a/src/movie.cpp b/src/movie.cpp index 78d509b4..c50598ea 100644 --- a/src/movie.cpp +++ b/src/movie.cpp @@ -114,7 +114,7 @@ SFORMAT FCEUMOV_STATEINFO[]={ { 0 } }; -char curMovieFilename[512] = {0}; +std::string curMovieFilename; MovieData currMovieData; MovieData defaultMovieData; int currRerecordCount; // Keep the global value @@ -807,7 +807,7 @@ static EMUFILE *openRecordingMovie(const char* fname) FCEU_PrintError("Error opening movie output file: %s", fname); return NULL; } - strcpy(curMovieFilename, fname); + curMovieFilename.assign(fname); return osRecordingMovie; } @@ -827,7 +827,7 @@ static void RedumpWholeMovieFile(bool justToggledRecording = false) bool recording = (movieMode == MOVIEMODE_RECORD); assert((NULL != osRecordingMovie) == (recording != justToggledRecording) && "osRecordingMovie should be consistent with movie mode!"); - if (NULL == openRecordingMovie(curMovieFilename)) + if (NULL == openRecordingMovie(curMovieFilename.c_str())) return; currMovieData.dump(osRecordingMovie, false/*currMovieData.binaryFlag*/, recording); @@ -875,7 +875,7 @@ static void OnMovieClosed() { assert(movieMode == MOVIEMODE_INACTIVE); - curMovieFilename[0] = 0; //No longer a current movie filename + curMovieFilename.clear(); //No longer a current movie filename freshMovie = false; //No longer a fresh movie loaded if (bindSavestate) AutoSS = false; //If bind movies to savestates is true, then there is no longer a valid auto-save to load @@ -1040,7 +1040,7 @@ bool FCEUI_LoadMovie(const char *fname, bool _read_only, int _pauseframe) currMovieData = MovieData(); - strcpy(curMovieFilename, fname); + curMovieFilename.assign(fname); FCEUFILE *fp = FCEU_fopen(fname,0,"rb",0); if (!fp) return false; if(fp->isArchive() && !_read_only) { @@ -1445,7 +1445,7 @@ bool FCEUMOV_ReadState(EMUFILE* is, uint32 size) #endif movie_readonly = true; } - if (FCEU_isFileInArchive(curMovieFilename)) + if (FCEU_isFileInArchive(curMovieFilename.c_str())) { //a little rule: cant load states in read+write mode with a movie from an archive. //so we are going to switch it to readonly mode in that case @@ -1956,7 +1956,7 @@ void FCEUI_MoviePlayFromBeginning(void) #endif } -string FCEUI_GetMovieName(void) +std::string FCEUI_GetMovieName(void) { return curMovieFilename; } @@ -2050,9 +2050,9 @@ void FCEUI_CreateMovieFile(std::string fn) void FCEUI_MakeBackupMovie(bool dispMessage) { //This function generates backup movie files - string currentFn; //Current movie fillename - string backupFn; //Target backup filename - string tempFn; //temp used in back filename creation + std::string currentFn; //Current movie fillename + std::string backupFn; //Target backup filename + std::string tempFn; //temp used in back filename creation stringstream stream; int x; //Temp variable for string manip bool exist = false; //Used to test if filename exists diff --git a/src/movie.h b/src/movie.h index dafccb0e..531f1dc6 100644 --- a/src/movie.h +++ b/src/movie.h @@ -276,7 +276,7 @@ private: extern EMOVIEMODE movieMode; extern MovieData currMovieData; extern int currFrameCounter; -extern char curMovieFilename[512]; +extern std::string curMovieFilename; extern bool subtitlesOnAVI; extern bool freshMovie; extern bool movie_readonly;