diff --git a/regamedll/CMakeLists.txt b/regamedll/CMakeLists.txt index 680de68c2..b06fbde60 100644 --- a/regamedll/CMakeLists.txt +++ b/regamedll/CMakeLists.txt @@ -337,6 +337,7 @@ set(GAMEDLL_SRCS set(UNITTESTS_SRCS "unittests/animation_tests.cpp" + "unittests/shared_util_tests.cpp" "unittests/struct_offsets_tests.cpp" "unittests/TestRunner.cpp" ) diff --git a/regamedll/dlls/player.cpp b/regamedll/dlls/player.cpp index 90572ada2..7eb8f7878 100644 --- a/regamedll/dlls/player.cpp +++ b/regamedll/dlls/player.cpp @@ -9851,7 +9851,7 @@ void CBasePlayer::PrioritizeAutoBuyString(char (&autobuyString)[MAX_AUTOBUY_LENG int i = 0; // get the next token from the priority string. - while (*priorityChar != '\0' && *priorityChar != ' ') + while (*priorityChar != '\0' && *priorityChar != ' ' && i < (int)sizeof(priorityToken) - 1) { priorityToken[i++] = *priorityChar; priorityChar++; @@ -9859,6 +9859,10 @@ void CBasePlayer::PrioritizeAutoBuyString(char (&autobuyString)[MAX_AUTOBUY_LENG priorityToken[i] = '\0'; + // skip the rest of an oversized token that did not fit + while (*priorityChar != '\0' && *priorityChar != ' ') + priorityChar++; + // skip spaces while (*priorityChar == ' ') priorityChar++; diff --git a/regamedll/game_shared/shared_util.cpp b/regamedll/game_shared/shared_util.cpp index c7a2a19dc..380c4d29f 100644 --- a/regamedll/game_shared/shared_util.cpp +++ b/regamedll/game_shared/shared_util.cpp @@ -162,14 +162,17 @@ char *SharedParse(char *data) return data; } - s_shared_token[len++] = c; + // prevent overflow of the fixed-size token buffer + if (len < (int)sizeof(s_shared_token) - 1) + s_shared_token[len++] = c; } } // parse single characters if (c == '{' || c == '}'|| c == ')'|| c == '(' || c == '\'' || c == ',') { - s_shared_token[len++] = c; + if (len < (int)sizeof(s_shared_token) - 1) + s_shared_token[len++] = c; s_shared_token[len] = '\0'; return data + 1; } @@ -177,9 +180,9 @@ char *SharedParse(char *data) // parse a regular word do { - s_shared_token[len] = c; + if (len < (int)sizeof(s_shared_token) - 1) + s_shared_token[len++] = c; data++; - len++; c = *data; if (c == '{' || c == '}'|| c == ')'|| c == '(' || c == '\'' || c == ',') diff --git a/regamedll/msvc/ReGameDLL.vcxproj b/regamedll/msvc/ReGameDLL.vcxproj index 80e93c922..1aa8081b5 100644 --- a/regamedll/msvc/ReGameDLL.vcxproj +++ b/regamedll/msvc/ReGameDLL.vcxproj @@ -579,6 +579,12 @@ true true + + true + true + true + true + true true diff --git a/regamedll/unittests/shared_util_tests.cpp b/regamedll/unittests/shared_util_tests.cpp new file mode 100644 index 000000000..a60a96f0b --- /dev/null +++ b/regamedll/unittests/shared_util_tests.cpp @@ -0,0 +1,47 @@ +#include "precompiled.h" +#include "cppunitlite/TestHarness.h" + +// Regression test for the SharedParse() global-buffer overflow. +// s_shared_token is char[1500]; SharedParse() must never emit a token that +// does not fit, regardless of the size of a single unquoted/quoted token in +// the input. On the unpatched code the oversized-token cases below smash the +// global buffer (crash / memory corruption) before the CHECK is even reached. +const int SHARED_TOKEN_SIZE = 1500; // must match char s_shared_token[1500] + +TEST(RegularWordOverflow, SharedParse, 10) +{ + // One unquoted "word" of 5000 non-space characters (no quotes/specials). + static char input[5001]; + Q_memset(input, 'A', sizeof(input) - 1); + input[sizeof(input) - 1] = '\0'; + + SharedParse(input); + CHECK("oversized word must be truncated", Q_strlen(SharedGetToken()) < SHARED_TOKEN_SIZE); +} + +TEST(QuotedStringOverflow, SharedParse, 10) +{ + // One quoted string of 5000 characters between the quotes. + static char input[5003]; + input[0] = '\"'; + Q_memset(input + 1, 'B', 5000); + input[5001] = '\"'; + input[5002] = '\0'; + + SharedParse(input); + CHECK("oversized quoted string must be truncated", Q_strlen(SharedGetToken()) < SHARED_TOKEN_SIZE); +} + +TEST(NormalTokensStillWork, SharedParse, 10) +{ + // The fix must not change behavior for well-formed input. + char input[] = " primaryWeapon \"hello world\" , "; + char *p = SharedParse(input); + CHECK("plain word token", Q_strcmp(SharedGetToken(), "primaryWeapon") == 0); + + p = SharedParse(p); + CHECK("quoted string token", Q_strcmp(SharedGetToken(), "hello world") == 0); + + p = SharedParse(p); + CHECK("comma token", Q_strcmp(SharedGetToken(), ",") == 0); +}