From 3eb9c808c1f24479a7aa967354ece06f3c4e3173 Mon Sep 17 00:00:00 2001 From: Ben Hymers Date: Mon, 14 Mar 2016 09:33:17 +0000 Subject: [PATCH] Fix many cases of return values from ftell and fseek being ignored These could lead to undefined behaviour if e.g. a negative value from ftell was used to allocate memory. Also store result of ftell in a long; The result may previously have been truncated on some platforms --- RecastDemo/Source/InputGeom.cpp | 22 +++++++++++++++++++--- RecastDemo/Source/MeshLoaderObj.cpp | 19 ++++++++++++++++--- RecastDemo/Source/TestCase.cpp | 19 ++++++++++++++++--- RecastDemo/Source/imguiRenderGL.cpp | 21 +++++++++++++++++---- 4 files changed, 68 insertions(+), 13 deletions(-) diff --git a/RecastDemo/Source/InputGeom.cpp b/RecastDemo/Source/InputGeom.cpp index a0ec2c2..3d08855 100644 --- a/RecastDemo/Source/InputGeom.cpp +++ b/RecastDemo/Source/InputGeom.cpp @@ -166,10 +166,26 @@ bool InputGeom::loadGeomSet(rcContext* ctx, const std::string& filepath) char* buf = 0; FILE* fp = fopen(filepath.c_str(), "rb"); if (!fp) + { return false; - fseek(fp, 0, SEEK_END); - int bufSize = ftell(fp); - fseek(fp, 0, SEEK_SET); + } + if (fseek(fp, 0, SEEK_END) != 0) + { + fclose(fp); + return false; + } + + long bufSize = ftell(fp); + if (bufSize < 0) + { + fclose(fp); + return false; + } + if (fseek(fp, 0, SEEK_SET) != 0) + { + fclose(fp); + return false; + } buf = new char[bufSize]; if (!buf) { diff --git a/RecastDemo/Source/MeshLoaderObj.cpp b/RecastDemo/Source/MeshLoaderObj.cpp index 66f4d7f..8b44127 100644 --- a/RecastDemo/Source/MeshLoaderObj.cpp +++ b/RecastDemo/Source/MeshLoaderObj.cpp @@ -141,9 +141,22 @@ bool rcMeshLoaderObj::load(const std::string& filename) FILE* fp = fopen(filename.c_str(), "rb"); if (!fp) return false; - fseek(fp, 0, SEEK_END); - int bufSize = ftell(fp); - fseek(fp, 0, SEEK_SET); + if (fseek(fp, 0, SEEK_END) != 0) + { + fclose(fp); + return false; + } + long bufSize = ftell(fp); + if (bufSize < 0) + { + fclose(fp); + return false; + } + if (fseek(fp, 0, SEEK_SET) != 0) + { + fclose(fp); + return false; + } buf = new char[bufSize]; if (!buf) { diff --git a/RecastDemo/Source/TestCase.cpp b/RecastDemo/Source/TestCase.cpp index 7c447a3..aecf9a8 100644 --- a/RecastDemo/Source/TestCase.cpp +++ b/RecastDemo/Source/TestCase.cpp @@ -102,9 +102,22 @@ bool TestCase::load(const std::string& filePath) FILE* fp = fopen(filePath.c_str(), "rb"); if (!fp) return false; - fseek(fp, 0, SEEK_END); - int bufSize = ftell(fp); - fseek(fp, 0, SEEK_SET); + if (fseek(fp, 0, SEEK_END) != 0) + { + fclose(fp); + return false; + } + long bufSize = ftell(fp); + if (bufSize < 0) + { + fclose(fp); + return false; + } + if (fseek(fp, 0, SEEK_SET) != 0) + { + fclose(fp); + return false; + } buf = new char[bufSize]; if (!buf) { diff --git a/RecastDemo/Source/imguiRenderGL.cpp b/RecastDemo/Source/imguiRenderGL.cpp index a595964..9f05375 100644 --- a/RecastDemo/Source/imguiRenderGL.cpp +++ b/RecastDemo/Source/imguiRenderGL.cpp @@ -247,9 +247,22 @@ bool imguiRenderGLInit(const char* fontpath) // Load font. FILE* fp = fopen(fontpath, "rb"); if (!fp) return false; - fseek(fp, 0, SEEK_END); - size_t size = ftell(fp); - fseek(fp, 0, SEEK_SET); + if (fseek(fp, 0, SEEK_END) != 0) + { + fclose(fp); + return false; + } + long size = ftell(fp); + if (size < 0) + { + fclose(fp); + return false; + } + if (fseek(fp, 0, SEEK_SET) != 0) + { + fclose(fp); + return false; + } unsigned char* ttfBuffer = (unsigned char*)malloc(size); if (!ttfBuffer) @@ -260,7 +273,7 @@ bool imguiRenderGLInit(const char* fontpath) size_t readLen = fread(ttfBuffer, 1, size, fp); fclose(fp); - if (readLen != size) + if (readLen != static_cast(size)) { free(ttfBuffer); return false;