Fixed buffer overrun when opening a recent file (fixes #558)

This commit is contained in:
Damien GERARD 2015-08-06 13:49:14 +02:00
parent 95b2ada22f
commit 4a20a4c412
3 changed files with 26 additions and 28 deletions

View File

@ -223,7 +223,7 @@ public:
//! \name File Operations //! \name File Operations
//@{ //@{
//The doXXX functions apply to a single buffer and dont need to worry about views, with the excpetion of doClose, since closing one view doesnt have to mean the document is gone //The doXXX functions apply to a single buffer and dont need to worry about views, with the excpetion of doClose, since closing one view doesnt have to mean the document is gone
BufferID doOpen(const TCHAR *fileName, bool isRecursive = false, bool isReadOnly = false, int encoding = -1, const TCHAR *backupFileName = NULL, time_t fileNameTimestamp = 0); BufferID doOpen(const generic_string& fileName, bool isRecursive = false, bool isReadOnly = false, int encoding = -1, const TCHAR *backupFileName = NULL, time_t fileNameTimestamp = 0);
bool doReload(BufferID id, bool alert = true); bool doReload(BufferID id, bool alert = true);
bool doSave(BufferID, const TCHAR * filename, bool isSaveCopy = false); bool doSave(BufferID, const TCHAR * filename, bool isSaveCopy = false);
void doClose(BufferID, int whichOne, bool doDeleteBackup = false); void doClose(BufferID, int whichOne, bool doDeleteBackup = false);

View File

@ -2154,7 +2154,7 @@ void Notepad_plus::command(int id)
MB_OK|MB_APPLMODAL); MB_OK|MB_APPLMODAL);
} }
NppParameters *pNppParams = NppParameters::getInstance(); NppParameters *pNppParams = NppParameters::getInstance();
BufferID bufID = doOpen((pNppParams->getContextMenuPath()).c_str()); BufferID bufID = doOpen((pNppParams->getContextMenuPath()));
switchToFile(bufID); switchToFile(bufID);
break; break;
} }
@ -2472,7 +2472,7 @@ void Notepad_plus::command(int id)
int size = _lastRecentFileList.getSize(); int size = _lastRecentFileList.getSize();
for (int i = size - 1; i >= 0; i--) for (int i = size - 1; i >= 0; i--)
{ {
BufferID test = doOpen(_lastRecentFileList.getIndex(i).c_str()); BufferID test = doOpen(_lastRecentFileList.getIndex(i));
if (test != BUFFER_INVALID) if (test != BUFFER_INVALID)
lastOne = test; lastOne = test;
} }
@ -2570,13 +2570,11 @@ void Notepad_plus::command(int id)
case IDM_FILE_RESTORELASTCLOSEDFILE: case IDM_FILE_RESTORELASTCLOSEDFILE:
{ {
generic_string lastOpenedFullPath = _lastRecentFileList.getFirstItem(); generic_string lastOpenedFullPath = _lastRecentFileList.getFirstItem();
if (lastOpenedFullPath != TEXT("")) if (not lastOpenedFullPath.empty())
{ {
BufferID lastOpened = doOpen(lastOpenedFullPath.c_str()); BufferID lastOpened = doOpen(lastOpenedFullPath);
if (lastOpened != BUFFER_INVALID) if (lastOpened != BUFFER_INVALID)
{
switchToFile(lastOpened); switchToFile(lastOpened);
}
} }
} }
break; break;
@ -2667,7 +2665,7 @@ void Notepad_plus::command(int id)
default : default :
if (id > IDM_FILEMENU_LASTONE && id < (IDM_FILEMENU_LASTONE + _lastRecentFileList.getMaxNbLRF() + 1)) if (id > IDM_FILEMENU_LASTONE && id < (IDM_FILEMENU_LASTONE + _lastRecentFileList.getMaxNbLRF() + 1))
{ {
BufferID lastOpened = doOpen(_lastRecentFileList.getItem(id).c_str()); BufferID lastOpened = doOpen(_lastRecentFileList.getItem(id));
if (lastOpened != BUFFER_INVALID) if (lastOpened != BUFFER_INVALID)
{ {
switchToFile(lastOpened); switchToFile(lastOpened);

View File

@ -37,10 +37,11 @@
using namespace std; using namespace std;
BufferID Notepad_plus::doOpen(const TCHAR *fileName, bool isRecursive, bool isReadOnly, int encoding, const TCHAR *backupFileName, time_t fileNameTimestamp) BufferID Notepad_plus::doOpen(const generic_string& fileName, bool isRecursive, bool isReadOnly, int encoding, const TCHAR *backupFileName, time_t fileNameTimestamp)
{ {
const rsize_t longFileNameBufferSize = MAX_PATH; // TODO stop using fixed-size buffer
const rsize_t longFileNameBufferSize = MAX_PATH; if (fileName.size() >= longFileNameBufferSize - 1) // issue with all other sub-routines
return BUFFER_INVALID;
//If [GetFullPathName] succeeds, the return value is the length, in TCHARs, of the string copied to lpBuffer, not including the terminating null character. //If [GetFullPathName] succeeds, the return value is the length, in TCHARs, of the string copied to lpBuffer, not including the terminating null character.
//If the lpBuffer buffer is too small to contain the path, the return value [of GetFullPathName] is the size, in TCHARs, of the buffer that is required to hold the path and the terminating null character. //If the lpBuffer buffer is too small to contain the path, the return value [of GetFullPathName] is the size, in TCHARs, of the buffer that is required to hold the path and the terminating null character.
@ -49,7 +50,7 @@ BufferID Notepad_plus::doOpen(const TCHAR *fileName, bool isRecursive, bool isRe
NppParameters *pNppParam = NppParameters::getInstance(); NppParameters *pNppParam = NppParameters::getInstance();
TCHAR longFileName[longFileNameBufferSize]; TCHAR longFileName[longFileNameBufferSize];
const DWORD getFullPathNameResult = ::GetFullPathName(fileName, longFileNameBufferSize, longFileName, NULL); const DWORD getFullPathNameResult = ::GetFullPathName(fileName.c_str(), longFileNameBufferSize, longFileName, NULL);
if ( getFullPathNameResult == 0 ) if ( getFullPathNameResult == 0 )
{ {
return BUFFER_INVALID; return BUFFER_INVALID;
@ -66,14 +67,14 @@ BufferID Notepad_plus::doOpen(const TCHAR *fileName, bool isRecursive, bool isRe
bool isSnapshotMode = backupFileName != NULL && PathFileExists(backupFileName); bool isSnapshotMode = backupFileName != NULL && PathFileExists(backupFileName);
if (isSnapshotMode && !PathFileExists(longFileName)) // UNTITLED if (isSnapshotMode && !PathFileExists(longFileName)) // UNTITLED
{ {
lstrcpy(longFileName, fileName); lstrcpy(longFileName, fileName.c_str());
} }
_lastRecentFileList.remove(longFileName); _lastRecentFileList.remove(longFileName);
const TCHAR * fileName2Find; generic_string fileName2Find;
generic_string gs_fileName = fileName; generic_string gs_fileName{fileName};
size_t res = gs_fileName.find_first_of(UNTITLED_STR); size_t res = gs_fileName.find_first_of(UNTITLED_STR);
if (res != string::npos && res == 0) if (res != string::npos && res == 0)
@ -85,7 +86,7 @@ BufferID Notepad_plus::doOpen(const TCHAR *fileName, bool isRecursive, bool isRe
fileName2Find = longFileName; fileName2Find = longFileName;
} }
BufferID test = MainFileManager->getBufferFromName(fileName2Find); BufferID test = MainFileManager->getBufferFromName(fileName2Find.c_str());
if (test != BUFFER_INVALID && !isSnapshotMode) if (test != BUFFER_INVALID && !isSnapshotMode)
{ {
//switchToFile(test); //switchToFile(test);
@ -248,14 +249,14 @@ BufferID Notepad_plus::doOpen(const TCHAR *fileName, bool isRecursive, bool isRe
} }
else else
{ {
if (globbing || ::PathIsDirectory(fileName)) if (globbing || ::PathIsDirectory(fileName.c_str()))
{ {
vector<generic_string> fileNames; vector<generic_string> fileNames;
vector<generic_string> patterns; vector<generic_string> patterns;
if (globbing) if (globbing)
{ {
const TCHAR * substring = wcsrchr(fileName, TCHAR('\\')); const TCHAR * substring = wcsrchr(fileName.c_str(), TCHAR('\\'));
size_t pos = substring - fileName; size_t pos = substring - fileName.c_str();
patterns.push_back(substring + 1); patterns.push_back(substring + 1);
generic_string dir(fileName, pos + 1); generic_string dir(fileName, pos + 1);
@ -264,7 +265,7 @@ BufferID Notepad_plus::doOpen(const TCHAR *fileName, bool isRecursive, bool isRe
else else
{ {
generic_string fileNameStr = fileName; generic_string fileNameStr = fileName;
if (fileName[lstrlen(fileName) - 1] != '\\') if (fileName[fileName.size() - 1] != '\\')
fileNameStr += TEXT("\\"); fileNameStr += TEXT("\\");
patterns.push_back(TEXT("*")); patterns.push_back(TEXT("*"));
@ -277,19 +278,17 @@ BufferID Notepad_plus::doOpen(const TCHAR *fileName, bool isRecursive, bool isRe
if (nbFiles2Open > 200) if (nbFiles2Open > 200)
{ {
ok2Open = IDYES == _nativeLangSpeaker.messageBox("NbFileToOpenImportantWarning", ok2Open = IDYES == _nativeLangSpeaker.messageBox("NbFileToOpenImportantWarning",
_pPublicInterface->getHSelf(), _pPublicInterface->getHSelf(),
TEXT("$INT_REPLACE$ files are about to be opened.\rAre you sure to open them?"), TEXT("$INT_REPLACE$ files are about to be opened.\rAre you sure to open them?"),
TEXT("Amount of files to open is too large"), TEXT("Amount of files to open is too large"),
MB_YESNO|MB_APPLMODAL, MB_YESNO|MB_APPLMODAL,
nbFiles2Open); nbFiles2Open);
} }
if (ok2Open) if (ok2Open)
{ {
for (size_t i = 0 ; i < nbFiles2Open ; ++i) for (size_t i = 0 ; i < nbFiles2Open ; ++i)
{ doOpen(fileNames[i]);
doOpen(fileNames[i].c_str());
}
} }
} }
else else
@ -313,6 +312,7 @@ BufferID Notepad_plus::doOpen(const TCHAR *fileName, bool isRecursive, bool isRe
return buffer; return buffer;
} }
bool Notepad_plus::doReload(BufferID id, bool alert) bool Notepad_plus::doReload(BufferID id, bool alert)
{ {
if (alert) if (alert)