From cb6de6a38482375cf10f1411f52885bd75ec2e2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Sat, 4 Feb 2023 00:33:56 +0100 Subject: [PATCH 1/4] Group and order `mem_*` function implementations --- src/base/system.cpp | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/base/system.cpp b/src/base/system.cpp index a65651211..25a893d6a 100644 --- a/src/base/system.cpp +++ b/src/base/system.cpp @@ -223,6 +223,25 @@ void mem_zero(void *block, unsigned size) memset(block, 0, size); } +int mem_comp(const void *a, const void *b, int size) +{ + return memcmp(a, b, size); +} + +int mem_has_null(const void *block, unsigned size) +{ + const unsigned char *bytes = (const unsigned char *)block; + unsigned i; + for(i = 0; i < size; i++) + { + if(bytes[i] == 0) + { + return 1; + } + } + return 0; +} + IOHANDLE io_open_impl(const char *filename, int flags) { dbg_assert(flags == (IOFLAG_READ | IOFLAG_SKIP_BOM) || flags == IOFLAG_READ || flags == IOFLAG_WRITE || flags == IOFLAG_APPEND, "flags must be read, read+skipbom, write or append"); @@ -3426,25 +3445,6 @@ void str_escape(char **dst, const char *src, const char *end) **dst = 0; } -int mem_comp(const void *a, const void *b, int size) -{ - return memcmp(a, b, size); -} - -int mem_has_null(const void *block, unsigned size) -{ - const unsigned char *bytes = (const unsigned char *)block; - unsigned i; - for(i = 0; i < size; i++) - { - if(bytes[i] == 0) - { - return 1; - } - } - return 0; -} - void net_stats(NETSTATS *stats_inout) { *stats_inout = network_stats; From d4809fa9a8e7327a2c77b879578c9711aec1ffa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Sat, 4 Feb 2023 00:36:26 +0100 Subject: [PATCH 2/4] Change `mem_has_null` return type to `bool` --- src/base/system.cpp | 6 +++--- src/base/system.h | 5 ++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/base/system.cpp b/src/base/system.cpp index 25a893d6a..4d2a64ae5 100644 --- a/src/base/system.cpp +++ b/src/base/system.cpp @@ -228,7 +228,7 @@ int mem_comp(const void *a, const void *b, int size) return memcmp(a, b, size); } -int mem_has_null(const void *block, unsigned size) +bool mem_has_null(const void *block, unsigned size) { const unsigned char *bytes = (const unsigned char *)block; unsigned i; @@ -236,10 +236,10 @@ int mem_has_null(const void *block, unsigned size) { if(bytes[i] == 0) { - return 1; + return true; } } - return 0; + return false; } IOHANDLE io_open_impl(const char *filename, int flags) diff --git a/src/base/system.h b/src/base/system.h index 6ffcb1d6e..998cfb789 100644 --- a/src/base/system.h +++ b/src/base/system.h @@ -181,10 +181,9 @@ int mem_comp(const void *a, const void *b, int size); * @param block Pointer to the block to check for nulls. * @param size Size of the block. * - * @return 1 - The block has a null byte. - * @return 0 - The block does not have a null byte. + * @return true if the block has a null byte, false otherwise. */ -int mem_has_null(const void *block, unsigned size); +bool mem_has_null(const void *block, unsigned size); /** * @defgroup File-IO From 18cfbb53f9824dc35b07750302f310d1e3f66b63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Sat, 4 Feb 2023 00:39:03 +0100 Subject: [PATCH 3/4] Use `size_t` for `mem_*` function size parameters --- src/base/system.cpp | 13 ++++++------- src/base/system.h | 10 +++++----- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/base/system.cpp b/src/base/system.cpp index 4d2a64ae5..3e70e8d68 100644 --- a/src/base/system.cpp +++ b/src/base/system.cpp @@ -208,31 +208,30 @@ void dbg_msg(const char *sys, const char *fmt, ...) /* */ -void mem_copy(void *dest, const void *source, unsigned size) +void mem_copy(void *dest, const void *source, size_t size) { memcpy(dest, source, size); } -void mem_move(void *dest, const void *source, unsigned size) +void mem_move(void *dest, const void *source, size_t size) { memmove(dest, source, size); } -void mem_zero(void *block, unsigned size) +void mem_zero(void *block, size_t size) { memset(block, 0, size); } -int mem_comp(const void *a, const void *b, int size) +int mem_comp(const void *a, const void *b, size_t size) { return memcmp(a, b, size); } -bool mem_has_null(const void *block, unsigned size) +bool mem_has_null(const void *block, size_t size) { const unsigned char *bytes = (const unsigned char *)block; - unsigned i; - for(i = 0; i < size; i++) + for(size_t i = 0; i < size; i++) { if(bytes[i] == 0) { diff --git a/src/base/system.h b/src/base/system.h index 998cfb789..4b2a3ea7c 100644 --- a/src/base/system.h +++ b/src/base/system.h @@ -131,7 +131,7 @@ void dbg_msg(const char *sys, const char *fmt, ...) * * @see mem_move */ -void mem_copy(void *dest, const void *source, unsigned size); +void mem_copy(void *dest, const void *source, size_t size); /** * Copies a a memory block. @@ -146,7 +146,7 @@ void mem_copy(void *dest, const void *source, unsigned size); * * @see mem_copy */ -void mem_move(void *dest, const void *source, unsigned size); +void mem_move(void *dest, const void *source, size_t size); /** * Sets a complete memory block to 0. @@ -156,7 +156,7 @@ void mem_move(void *dest, const void *source, unsigned size); * @param block Pointer to the block to zero out. * @param size Size of the block. */ -void mem_zero(void *block, unsigned size); +void mem_zero(void *block, size_t size); /** * Compares two blocks of memory @@ -171,7 +171,7 @@ void mem_zero(void *block, unsigned size); * @return 0 - Block a is equal to block b. * @return > 0 - Block a is greater than block b. */ -int mem_comp(const void *a, const void *b, int size); +int mem_comp(const void *a, const void *b, size_t size); /** * Checks whether a block of memory contains null bytes. @@ -183,7 +183,7 @@ int mem_comp(const void *a, const void *b, int size); * * @return true if the block has a null byte, false otherwise. */ -bool mem_has_null(const void *block, unsigned size); +bool mem_has_null(const void *block, size_t size); /** * @defgroup File-IO From ec7f5560a36f10aefc4e90646dc9cbe50ac1d8ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Sun, 19 Feb 2023 13:45:48 +0100 Subject: [PATCH 4/4] Fix possible integer overflows using `mem_*` functions --- src/engine/client/text.cpp | 2 +- src/game/editor/io.cpp | 5 +++-- src/tools/map_optimize.cpp | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/engine/client/text.cpp b/src/engine/client/text.cpp index 443e624d8..3c8e0df41 100644 --- a/src/engine/client/text.cpp +++ b/src/engine/client/text.cpp @@ -540,7 +540,7 @@ class CTextRender : public IEngineTextRender if(Width > 0 && Height > 0) { // prepare glyph data - mem_zero(ms_aGlyphData, Width * Height); + mem_zero(ms_aGlyphData, (size_t)Width * Height); for(py = 0; py < pBitmap->rows; py++) for(px = 0; px < pBitmap->width; px++) diff --git a/src/game/editor/io.cpp b/src/game/editor/io.cpp index b5fd115a5..63eebb7a9 100644 --- a/src/game/editor/io.cpp +++ b/src/game/editor/io.cpp @@ -511,8 +511,9 @@ bool CEditorMap::Load(class IStorage *pStorage, const char *pFileName, int Stora // copy image data void *pData = DataFile.GetData(pItem->m_ImageData); - pImg->m_pData = malloc((size_t)pImg->m_Width * pImg->m_Height * 4); - mem_copy(pImg->m_pData, pData, pImg->m_Width * pImg->m_Height * 4); + const size_t DataSize = (size_t)pImg->m_Width * pImg->m_Height * 4; + pImg->m_pData = malloc(DataSize); + mem_copy(pImg->m_pData, pData, DataSize); int TextureLoadFlag = m_pEditor->Graphics()->HasTextureArrays() ? IGraphics::TEXLOAD_TO_2D_ARRAY_TEXTURE : IGraphics::TEXLOAD_TO_3D_TEXTURE; if(pImg->m_Width % 16 != 0 || pImg->m_Height % 16 != 0) TextureLoadFlag = 0; diff --git a/src/tools/map_optimize.cpp b/src/tools/map_optimize.cpp index 9602703ca..c0ac1b8bf 100644 --- a/src/tools/map_optimize.cpp +++ b/src/tools/map_optimize.cpp @@ -250,7 +250,7 @@ int main(int argc, const char **argv) } else if(aImageFlags[ImageIndex] == 0) { - mem_zero(pImgBuff, Width * Height * 4); + mem_zero(pImgBuff, (size_t)Width * Height * 4); } else {