Git commit 12860515871f2c4e54a55bc9de20ed55a7671507 by Julian Helfferich. Committed on 30/04/2016 at 20:01. Pushed by helfferich into branch 'master'. Fix Coverity #76255: Calling risky function When loading a plain text replay file, fscanf() was used. The use of this function is discouraged as it can lead to buffer overflows if the string read is larger than the reserved buffer. Now, the C-style functions are replaced with their C++ equivalents. This means the data is read from an ifstream and stored to a string via an istringstream. M +1 -0 KBlocksDefine.cpp M +1 -0 KBlocksDefine.h M +61 -34 KBlocksGameReplayer.cpp M +3 -3 KBlocksGameReplayer.h M +1 -1 KBlocksRepWin.cpp http://commits.kde.org/kblocks/12860515871f2c4e54a55bc9de20ed55a7671507 diff --git a/KBlocksDefine.cpp b/KBlocksDefine.cpp index 06a31f1..1bfcbed 100644 --- a/KBlocksDefine.cpp +++ b/KBlocksDefine.cpp @@ -11,6 +11,7 @@ = Q_LOGGING_CATEGORY(KBGeneral, "KBlocks.General") Q_LOGGING_CATEGORY(KBGraphics, "KBlocks.Graphics") +Q_LOGGING_CATEGORY(KBReplay, "KBlocks.Replay") Q_LOGGING_CATEGORY(KBSound, "KBlocks.Sound") = const char *KBlocksRecordText[RecordDataType_Max_Count] =3D { diff --git a/KBlocksDefine.h b/KBlocksDefine.h index f204cde..6c70bd2 100644 --- a/KBlocksDefine.h +++ b/KBlocksDefine.h @@ -14,6 +14,7 @@ #include Q_DECLARE_LOGGING_CATEGORY(KBGeneral) Q_DECLARE_LOGGING_CATEGORY(KBGraphics) +Q_DECLARE_LOGGING_CATEGORY(KBReplay) Q_DECLARE_LOGGING_CATEGORY(KBSound) = #define PREPARE_AREA_WIDTH 5 diff --git a/KBlocksGameReplayer.cpp b/KBlocksGameReplayer.cpp index bee82b1..6cb51fd 100644 --- a/KBlocksGameReplayer.cpp +++ b/KBlocksGameReplayer.cpp @@ -9,27 +9,50 @@ **************************************************************************= */ #include "KBlocksGameReplayer.h" = +#include +#include + +#include "KBlocksDefine.h" + KBlocksGameReplayer::KBlocksGameReplayer(const char *fileName, bool isBina= ryMode) { + // Map data types to strings for reading text file for (int i =3D 0; i < RecordDataType_Max_Count; ++i) { mRTMap[ KBlocksRecordText[i] ] =3D i; } mRTMap[string("MaxCount")] =3D -1; = - FILE *pFile =3D fopen(fileName, "r"); + // Set default variables in case loading the file fails + mGameCount =3D 0; + mGameSeed =3D 0; + mSameSeed =3D false; + mStepLength =3D 1; = - if (!pFile) { - mGameCount =3D 0; - mGameSeed =3D 0; - mSameSeed =3D false; - mStepLength =3D 1; + // Open replay file + std::ifstream replayFile; + if (isBinaryMode) { + replayFile.open(fileName, std::ios::binary); + } else { + replayFile.open(fileName); + } + + // Check that replay file was opened successfully + if (!replayFile.is_open()) { + qCWarning(KBReplay) << "Unable to open file " << fileName; return; } = if (isBinaryMode) { - loadBinary(pFile); + loadBinary(replayFile); } else { - loadText(pFile); + loadText(replayFile); + } + + // Check that more than two Replay steps have been loaded + // The two first steps set the required variables. + if (mReplayList.size() < 2) { + qCWarning(KBReplay) << "Problem loading replay file" << fileName; + return; } = mGameCount =3D mReplayList.front().value; @@ -38,9 +61,7 @@ KBlocksGameReplayer::KBlocksGameReplayer(const char *file= Name, bool isBinaryMode mSameSeed =3D (mReplayList.front().index =3D=3D 1); mReplayList.pop_front(); = - mStepLength =3D 1; - - fclose(pFile); + replayFile.close(); } = KBlocksGameReplayer::~KBlocksGameReplayer() @@ -96,48 +117,54 @@ bool KBlocksGameReplayer::getNextRecords(vector *data) return true; } = -void KBlocksGameReplayer::loadText(FILE *pFile) +void KBlocksGameReplayer::loadText(std::ifstream &replayFile) { - int count =3D 0; - char tmpString[256]; + std::string line; + std::istringstream inStream; + std::string tmpString; KBlocksReplayData tmpData; mReplayList.clear(); - while (1) { - count =3D fscanf(pFile, "%d %s %d %d", &(tmpData.time), tmpString,= &(tmpData.index), &(tmpData.value)); - tmpData.type =3D mRTMap[string(tmpString)]; - if ((tmpData.type =3D=3D -1) || (count !=3D 4)) { + do { + std::getline(replayFile, line); + inStream.str(line); + inStream >> tmpData.time >> tmpString >> tmpData.index >> tmpData.= value; + tmpData.type =3D mRTMap[tmpString]; + if ((tmpData.type =3D=3D -1) || inStream.fail()) { break; } mReplayList.push_back(tmpData); - } + inStream.clear(); + } while (!replayFile.eof()); } = -void KBlocksGameReplayer::loadBinary(FILE *pFile) +void KBlocksGameReplayer::loadBinary(std::ifstream &replayFile) { KBlocksReplayData tmpData; mReplayList.clear(); - tmpData.time =3D fgetc(pFile); - tmpData.type =3D fgetc(pFile); - tmpData.index =3D fgetc(pFile); - tmpData.value =3D fgetc(pFile); - while (tmpData.time !=3D EOF) { + + tmpData.time =3D replayFile.get(); + tmpData.type =3D replayFile.get(); + tmpData.index =3D replayFile.get(); + tmpData.value =3D replayFile.get(); + + do { if (tmpData.type =3D=3D RecordDataType_Skipped) { int tmpTime =3D tmpData.time; while (tmpData.type =3D=3D RecordDataType_Skipped) { - tmpData.time =3D fgetc(pFile); - tmpData.type =3D fgetc(pFile); - tmpData.index =3D fgetc(pFile); - tmpData.value =3D fgetc(pFile); + tmpData.time =3D replayFile.get(); + tmpData.type =3D replayFile.get(); + tmpData.index =3D replayFile.get(); + tmpData.value =3D replayFile.get(); = tmpTime +=3D tmpData.time; } tmpData.time =3D tmpTime; } mReplayList.push_back(tmpData); - tmpData.time =3D fgetc(pFile); - tmpData.type =3D fgetc(pFile); - tmpData.index =3D fgetc(pFile); - tmpData.value =3D fgetc(pFile); - } + tmpData.time =3D replayFile.get(); + tmpData.type =3D replayFile.get(); + tmpData.index =3D replayFile.get(); + tmpData.value =3D replayFile.get(); + } while (!replayFile.eof()); } = diff --git a/KBlocksGameReplayer.h b/KBlocksGameReplayer.h index 2782477..fc83e12 100644 --- a/KBlocksGameReplayer.h +++ b/KBlocksGameReplayer.h @@ -12,7 +12,7 @@ = #include "KBlocksDefine.h" = -#include +#include #include #include #include @@ -46,8 +46,8 @@ public: bool getNextRecords(vector *data); = private: - void loadText(FILE *pFile); - void loadBinary(FILE *pFile); + void loadText(std::ifstream &pFile); + void loadBinary(std::ifstream &pFile); = private: int mGameCount; diff --git a/KBlocksRepWin.cpp b/KBlocksRepWin.cpp index 5a7f17f..f4da381 100644 --- a/KBlocksRepWin.cpp +++ b/KBlocksRepWin.cpp @@ -160,7 +160,7 @@ void KBlocksRepWin::replayOneStep() { int tmpPieceChanged =3D 0; if (!mpGameLogic->playRecordOneStep(&tmpPieceChanged)) { - printf("Finished Replay!\n"); + qCDebug(KBReplay) << "Finished Replay!"; mUpdateTimer.stop(); } if (tmpPieceChanged !=3D 0) {