From ff25ddfef0230a752809b7b06a7a9a3cf0f00684 Mon Sep 17 00:00:00 2001 From: Gabriel Ravier Date: Thu, 2 Jul 2020 16:43:13 +0200 Subject: [PATCH 1/2] src/TextScr.cpp: Do not crash when TSC files are too big with FIX_BUGS Signed-off-by: Gabriel Ravier --- src/TextScr.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/TextScr.cpp b/src/TextScr.cpp index d7d6fe00..61d99263 100644 --- a/src/TextScr.cpp +++ b/src/TextScr.cpp @@ -172,6 +172,11 @@ BOOL LoadTextScript_Stage(const char *name) return FALSE; // Read Head.tsc. Note that head_size may exceed the size of 'gTS.data' (TSC_BUFFER_SIZE) +#ifdef FIX_BUGS + if (head_size > TSC_BUFFER_SIZE) // The original doesn't check for any kind of buffer overflow here, so feeding in a 1 MiB Head.tsc (assuming an unchanged TSC_BUFFER_SIZE) would be sure to crash the game, for example. + return FALSE; +#endif + fread(gTS.data, 1, head_size, fp); EncryptionBinaryData2((unsigned char*)gTS.data, head_size); gTS.data[head_size] = '\0'; @@ -189,6 +194,11 @@ BOOL LoadTextScript_Stage(const char *name) return FALSE; // Read stage's tsc. Note that head_size + body_size may exceed the size of 'gTS.data' (TSC_BUFFER_SIZE) +#ifdef FIX_BUGS + if ((head_size + body_size) > TSC_BUFFER_SIZE) // Same as above, the original doesn't bother checking and may crash on large enough input + return FALSE; +#endif + fread(&gTS.data[head_size], 1, body_size, fp); EncryptionBinaryData2((unsigned char*)&gTS.data[head_size], body_size); gTS.data[head_size + body_size] = '\0'; From 90de32a83d7fcd335a883dee2ae95848635f1633 Mon Sep 17 00:00:00 2001 From: Clownacy Date: Thu, 2 Jul 2020 15:52:37 +0100 Subject: [PATCH 2/2] Fix bugs in bugfix These checks would leak the FILE*. Also did some pedantic style tweaks. --- src/TextScr.cpp | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/TextScr.cpp b/src/TextScr.cpp index 61d99263..676422e9 100644 --- a/src/TextScr.cpp +++ b/src/TextScr.cpp @@ -167,16 +167,18 @@ BOOL LoadTextScript_Stage(const char *name) if (head_size == INVALID_FILE_SIZE) return FALSE; +#ifdef FIX_BUGS + // The original doesn't check for any kind of buffer overflow here, so feeding in a 1 MiB Head.tsc + // (assuming an unchanged TSC_BUFFER_SIZE) would be sure to crash the game, for example. + if (head_size > TSC_BUFFER_SIZE) + return FALSE; +#endif + fp = fopen(path, "rb"); if (fp == NULL) return FALSE; // Read Head.tsc. Note that head_size may exceed the size of 'gTS.data' (TSC_BUFFER_SIZE) -#ifdef FIX_BUGS - if (head_size > TSC_BUFFER_SIZE) // The original doesn't check for any kind of buffer overflow here, so feeding in a 1 MiB Head.tsc (assuming an unchanged TSC_BUFFER_SIZE) would be sure to crash the game, for example. - return FALSE; -#endif - fread(gTS.data, 1, head_size, fp); EncryptionBinaryData2((unsigned char*)gTS.data, head_size); gTS.data[head_size] = '\0'; @@ -189,16 +191,17 @@ BOOL LoadTextScript_Stage(const char *name) if (body_size == INVALID_FILE_SIZE) return FALSE; +#ifdef FIX_BUGS + // Same as above: the original doesn't bother checking, and may crash on large-enough input + if (head_size + body_size > TSC_BUFFER_SIZE) + return FALSE; +#endif + fp = fopen(path, "rb"); if (fp == NULL) return FALSE; // Read stage's tsc. Note that head_size + body_size may exceed the size of 'gTS.data' (TSC_BUFFER_SIZE) -#ifdef FIX_BUGS - if ((head_size + body_size) > TSC_BUFFER_SIZE) // Same as above, the original doesn't bother checking and may crash on large enough input - return FALSE; -#endif - fread(&gTS.data[head_size], 1, body_size, fp); EncryptionBinaryData2((unsigned char*)&gTS.data[head_size], body_size); gTS.data[head_size + body_size] = '\0';