From a8f3b5685028de488244f6221fc4d73993d472a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Sat, 3 Feb 2024 19:11:16 +0100 Subject: [PATCH] Fix undefined behavior on loading empty PNG files When empty PNG files are loaded, the `std::vector` for the file contents is resized to size 0, which results in undefined behavior when it is accessed with `front`. When `io_tell` fails, i.e. returns `-1`, this was incorrectly cast to an `unsigned` and therefore caused a very large allocation and potentially crashes due to lack of memory. --- src/engine/client/graphics_threaded.cpp | 8 +++++++- src/tools/dilate.cpp | 8 +++++++- src/tools/map_convert_07.cpp | 8 +++++++- src/tools/map_create_pixelart.cpp | 8 +++++++- src/tools/map_replace_image.cpp | 7 ++++++- 5 files changed, 34 insertions(+), 5 deletions(-) diff --git a/src/engine/client/graphics_threaded.cpp b/src/engine/client/graphics_threaded.cpp index 22f7a844d..05b1f44f5 100644 --- a/src/engine/client/graphics_threaded.cpp +++ b/src/engine/client/graphics_threaded.cpp @@ -584,7 +584,13 @@ bool CGraphics_Threaded::LoadPNG(CImageInfo *pImg, const char *pFilename, int St if(File) { io_seek(File, 0, IOSEEK_END); - unsigned int FileSize = io_tell(File); + long int FileSize = io_tell(File); + if(FileSize <= 0) + { + io_close(File); + log_error("game/png", "failed to get file size (%ld). filename='%s'", FileSize, pFilename); + return false; + } io_seek(File, 0, IOSEEK_START); TImageByteBuffer ByteBuffer; diff --git a/src/tools/dilate.cpp b/src/tools/dilate.cpp index 73d330874..4a8fcfb9b 100644 --- a/src/tools/dilate.cpp +++ b/src/tools/dilate.cpp @@ -12,7 +12,13 @@ int DilateFile(const char *pFilename) if(File) { io_seek(File, 0, IOSEEK_END); - unsigned int FileSize = io_tell(File); + long int FileSize = io_tell(File); + if(FileSize <= 0) + { + io_close(File); + dbg_msg("dilate", "failed to get file size (%ld). filename='%s'", FileSize, pFilename); + return false; + } io_seek(File, 0, IOSEEK_START); TImageByteBuffer ByteBuffer; SImageByteBuffer ImageByteBuffer(&ByteBuffer); diff --git a/src/tools/map_convert_07.cpp b/src/tools/map_convert_07.cpp index 6c0b8a162..699701658 100644 --- a/src/tools/map_convert_07.cpp +++ b/src/tools/map_convert_07.cpp @@ -31,7 +31,13 @@ int LoadPNG(CImageInfo *pImg, const char *pFilename) if(File) { io_seek(File, 0, IOSEEK_END); - unsigned int FileSize = io_tell(File); + long int FileSize = io_tell(File); + if(FileSize <= 0) + { + io_close(File); + dbg_msg("map_convert_07", "failed to get file size (%ld). filename='%s'", FileSize, pFilename); + return false; + } io_seek(File, 0, IOSEEK_START); TImageByteBuffer ByteBuffer; SImageByteBuffer ImageByteBuffer(&ByteBuffer); diff --git a/src/tools/map_create_pixelart.cpp b/src/tools/map_create_pixelart.cpp index 9689a847a..5d5e7edb4 100644 --- a/src/tools/map_create_pixelart.cpp +++ b/src/tools/map_create_pixelart.cpp @@ -319,7 +319,13 @@ bool LoadPNG(CImageInfo *pImg, const char *pFilename) } io_seek(File, 0, IOSEEK_END); - unsigned int FileSize = io_tell(File); + long int FileSize = io_tell(File); + if(FileSize <= 0) + { + io_close(File); + dbg_msg("map_create_pixelart", "ERROR: Failed to get file size (%ld). filename='%s'", FileSize, pFilename); + return false; + } io_seek(File, 0, IOSEEK_START); TImageByteBuffer ByteBuffer; SImageByteBuffer ImageByteBuffer(&ByteBuffer); diff --git a/src/tools/map_replace_image.cpp b/src/tools/map_replace_image.cpp index 1b19f39e9..f764f2449 100644 --- a/src/tools/map_replace_image.cpp +++ b/src/tools/map_replace_image.cpp @@ -29,7 +29,12 @@ bool LoadPNG(CImageInfo *pImg, const char *pFilename) if(File) { io_seek(File, 0, IOSEEK_END); - unsigned int FileSize = io_tell(File); + long int FileSize = io_tell(File); + if(FileSize <= 0) + { + io_close(File); + return false; + } io_seek(File, 0, IOSEEK_START); TImageByteBuffer ByteBuffer; SImageByteBuffer ImageByteBuffer(&ByteBuffer);