[openjk] 22/24: Add patches to fix bounds-checking in savegames, so valid saves are not rejected
Simon McVittie
smcv at debian.org
Sat Jan 21 21:44:03 UTC 2017
This is an automated email from the git hooks/post-receive script.
smcv pushed a commit to branch debian/master
in repository openjk.
commit 836b708f59f9a9dbe6b29b0c76029b0f464625fc
Author: Simon McVittie <smcv at debian.org>
Date: Sat Jan 21 15:32:04 2017 +0000
Add patches to fix bounds-checking in savegames, so valid saves are not rejected
---
debian/changelog | 2 +
...-bounds-check-when-loading-from-savegames.patch | 41 +++++++
...m-SOURCE_DATE_EPOCH-for-reproducible-buil.patch | 18 +--
...arm-debug-code-that-writes-to-c-nofreeent.patch | 2 +-
...are-too-long-raise-error-instead-of-assum.patch | 131 +++++++++++++++++++++
...oad-if-buffer-would-be-overflowed-don-t-j.patch | 67 +++++++++++
debian/patches/series | 3 +
7 files changed, 254 insertions(+), 10 deletions(-)
diff --git a/debian/changelog b/debian/changelog
index 89d74aa..333b408 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -2,6 +2,8 @@ openjk (0~20170112+dfsg1-1) UNRELEASED; urgency=medium
* New upstream snapshot
- Drop patches that were applied upstream
+ * Add patches to fix bounds-checking in savegames, so valid saves
+ are not rejected
-- Simon McVittie <smcv at debian.org> Sat, 21 Jan 2017 15:25:05 +0000
diff --git a/debian/patches/Icarus-fix-bounds-check-when-loading-from-savegames.patch b/debian/patches/Icarus-fix-bounds-check-when-loading-from-savegames.patch
new file mode 100644
index 0000000..0b3a127
--- /dev/null
+++ b/debian/patches/Icarus-fix-bounds-check-when-loading-from-savegames.patch
@@ -0,0 +1,41 @@
+From: Simon McVittie <smcv at debian.org>
+Date: Sat, 21 Jan 2017 15:06:40 +0000
+Subject: icarus: fix bounds check when loading from savegames
+
+m_byBuffer is a pointer to a fixed amount of dynamically allocated
+memory, not a statically allocated buffer, so sizeof() is the wrong
+tool here. Use the actual size of the buffer instead.
+
+Also, reading the full size of the buffer is fine, because we aren't
+going to append '\0' afterwards like we do in the other places
+touched by #881, so use > instead of >=.
+
+Bug: https://github.com/JACoders/OpenJK/pull/902
+Signed-off-by: Simon McVittie <smcv at debian.org>
+Forwarded: https://github.com/JACoders/OpenJK/pull/905
+---
+ code/icarus/IcarusImplementation.cpp | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/code/icarus/IcarusImplementation.cpp b/code/icarus/IcarusImplementation.cpp
+index 7d2fc8fc..cc893331 100644
+--- a/code/icarus/IcarusImplementation.cpp
++++ b/code/icarus/IcarusImplementation.cpp
+@@ -718,7 +718,7 @@ int CIcarus::Load()
+
+ int sg_buffer_size = saved_game.get_buffer_size();
+
+- if (sg_buffer_size < 0 || static_cast<size_t>(sg_buffer_size) >= sizeof(m_byBuffer))
++ if (sg_buffer_size < 0 || static_cast<size_t>(sg_buffer_size) > MAX_BUFFER_SIZE)
+ {
+ sg_buffer_size = 0;
+ }
+@@ -858,7 +858,7 @@ void CIcarus::BufferRead( void *pDstBuff, unsigned long ulNumBytesToRead )
+
+ int sg_buffer_size = saved_game.get_buffer_size();
+
+- if (sg_buffer_size < 0 || static_cast<size_t>(sg_buffer_size) >= sizeof(m_byBuffer))
++ if (sg_buffer_size < 0 || static_cast<size_t>(sg_buffer_size) > MAX_BUFFER_SIZE)
+ {
+ sg_buffer_size = 0;
+ }
diff --git a/debian/patches/Pick-up-date-from-SOURCE_DATE_EPOCH-for-reproducible-buil.patch b/debian/patches/Pick-up-date-from-SOURCE_DATE_EPOCH-for-reproducible-buil.patch
index d79aec0..cc660ec 100644
--- a/debian/patches/Pick-up-date-from-SOURCE_DATE_EPOCH-for-reproducible-buil.patch
+++ b/debian/patches/Pick-up-date-from-SOURCE_DATE_EPOCH-for-reproducible-buil.patch
@@ -30,7 +30,7 @@ Forwarded: https://github.com/JACoders/OpenJK/pull/874
8 files changed, 24 insertions(+), 10 deletions(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
-index 7a17d4d..e393e4b 100644
+index 7a17d4d7..e393e4b5 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -302,6 +302,15 @@ if(BuildPortableVersion)
@@ -50,7 +50,7 @@ index 7a17d4d..e393e4b 100644
diff --git a/code/game/g_main.cpp b/code/game/g_main.cpp
-index 8ae2c6d..6b385af 100644
+index 8ae2c6d4..6b385af3 100644
--- a/code/game/g_main.cpp
+++ b/code/game/g_main.cpp
@@ -617,7 +617,7 @@ void G_InitCvars( void ) {
@@ -72,7 +72,7 @@ index 8ae2c6d..6b385af 100644
srand( randomSeed );
diff --git a/code/qcommon/common.cpp b/code/qcommon/common.cpp
-index a1e73e4..031e2b3 100644
+index a1e73e49..031e2b3b 100644
--- a/code/qcommon/common.cpp
+++ b/code/qcommon/common.cpp
@@ -1050,7 +1050,7 @@ Com_Init
@@ -94,7 +94,7 @@ index a1e73e4..031e2b3 100644
#ifdef JK2_MODE
diff --git a/codeJK2/game/g_main.cpp b/codeJK2/game/g_main.cpp
-index ef78173..3dba32d 100644
+index ef781737..3dba32dd 100644
--- a/codeJK2/game/g_main.cpp
+++ b/codeJK2/game/g_main.cpp
@@ -558,7 +558,7 @@ void G_InitCvars( void ) {
@@ -116,7 +116,7 @@ index ef78173..3dba32d 100644
srand( randomSeed );
diff --git a/codemp/game/g_main.c b/codemp/game/g_main.c
-index e2a3e6b..f70f6e1 100644
+index e2a3e6bf..f70f6e17 100644
--- a/codemp/game/g_main.c
+++ b/codemp/game/g_main.c
@@ -193,7 +193,7 @@ void G_InitGame( int levelTime, int randomSeed, int restart ) {
@@ -129,7 +129,7 @@ index e2a3e6b..f70f6e1 100644
srand( randomSeed );
diff --git a/codemp/game/g_xcvar.h b/codemp/game/g_xcvar.h
-index ea1761c..ff0588b 100644
+index ea1761c7..ff0588bc 100644
--- a/codemp/game/g_xcvar.h
+++ b/codemp/game/g_xcvar.h
@@ -165,7 +165,7 @@ XCVAR_DEF( g_voteDelay, "3000", NULL, CVAR_NONE, qfalse )
@@ -142,7 +142,7 @@ index ea1761c..ff0588b 100644
XCVAR_DEF( pmove_fixed, "0", NULL, CVAR_SYSTEMINFO|CVAR_ARCHIVE, qtrue )
XCVAR_DEF( pmove_float, "0", NULL, CVAR_SYSTEMINFO|CVAR_ARCHIVE, qtrue )
diff --git a/codemp/qcommon/common.cpp b/codemp/qcommon/common.cpp
-index 525e3d4..3011c8d 100644
+index 525e3d44..3011c8de 100644
--- a/codemp/qcommon/common.cpp
+++ b/codemp/qcommon/common.cpp
@@ -1126,7 +1126,7 @@ void Com_Init( char *commandLine ) {
@@ -164,10 +164,10 @@ index 525e3d4..3011c8d 100644
SE_Init();
diff --git a/shared/qcommon/q_platform.h b/shared/qcommon/q_platform.h
-index 0d60290..9e1067a 100644
+index ee5392e8..4f9ee545 100644
--- a/shared/qcommon/q_platform.h
+++ b/shared/qcommon/q_platform.h
-@@ -362,3 +362,8 @@ typedef union byteAlias_u {
+@@ -358,3 +358,8 @@ typedef union byteAlias_u {
#else
#define PLATFORM_STRING OS_STRING "-" ARCH_STRING "-debug"
#endif
diff --git a/debian/patches/g_utils-disarm-debug-code-that-writes-to-c-nofreeent.patch b/debian/patches/g_utils-disarm-debug-code-that-writes-to-c-nofreeent.patch
index 48069d9..0d4fd49 100644
--- a/debian/patches/g_utils-disarm-debug-code-that-writes-to-c-nofreeent.patch
+++ b/debian/patches/g_utils-disarm-debug-code-that-writes-to-c-nofreeent.patch
@@ -12,7 +12,7 @@ Forwarded: no
1 file changed, 2 insertions(+)
diff --git a/code/game/g_utils.cpp b/code/game/g_utils.cpp
-index 9273fad..4d04c15 100644
+index 9273fad5..4d04c157 100644
--- a/code/game/g_utils.cpp
+++ b/code/game/g_utils.cpp
@@ -818,6 +818,7 @@ gentity_t *G_Spawn( void )
diff --git a/debian/patches/game-If-strings-are-too-long-raise-error-instead-of-assum.patch b/debian/patches/game-If-strings-are-too-long-raise-error-instead-of-assum.patch
new file mode 100644
index 0000000..e5d51a3
--- /dev/null
+++ b/debian/patches/game-If-strings-are-too-long-raise-error-instead-of-assum.patch
@@ -0,0 +1,131 @@
+From: Simon McVittie <smcv at debian.org>
+Date: Sat, 21 Jan 2017 15:12:50 +0000
+Subject: game: If strings are too long,
+ raise error instead of assuming 0 length
+
+Signed-off-by: Simon McVittie <smcv at debian.org>
+---
+ code/game/G_Timer.cpp | 14 ++++++--------
+ code/game/Q3_Interface.cpp | 6 +++---
+ code/game/g_roff.cpp | 4 +++-
+ codeJK2/game/Q3_Registers.cpp | 6 +++---
+ codeJK2/game/g_roff.cpp | 2 +-
+ 5 files changed, 16 insertions(+), 16 deletions(-)
+
+diff --git a/code/game/G_Timer.cpp b/code/game/G_Timer.cpp
+index 192594d1..89ecb94f 100644
+--- a/code/game/G_Timer.cpp
++++ b/code/game/G_Timer.cpp
+@@ -247,16 +247,14 @@ void TIMER_Load( void )
+
+ if (sg_buffer_size < 0 || static_cast<size_t>(sg_buffer_size) >= sizeof(tempBuffer))
+ {
+- sg_buffer_size = 0;
+- }
+- else
+- {
+- std::uninitialized_copy_n(
+- sg_buffer_data,
+- sg_buffer_size,
+- tempBuffer);
++ ::G_Error("invalid length for TMID string in saved game: %d\n", sg_buffer_size);
+ }
+
++ std::uninitialized_copy_n(
++ sg_buffer_data,
++ sg_buffer_size,
++ tempBuffer);
++
+ tempBuffer[sg_buffer_size] = '\0';
+
+ saved_game.read_chunk<int32_t>(
+diff --git a/code/game/Q3_Interface.cpp b/code/game/Q3_Interface.cpp
+index 2c740d48..585167a2 100644
+--- a/code/game/Q3_Interface.cpp
++++ b/code/game/Q3_Interface.cpp
+@@ -7328,7 +7328,7 @@ void CQuake3GameInterface::VariableLoadFloats( varFloat_m &fmap )
+
+ if (idSize < 0 || static_cast<size_t>(idSize) >= sizeof(tempBuffer))
+ {
+- idSize = 0;
++ ::G_Error("invalid length for FIDS string in save game: %d bytes\n", idSize);
+ }
+
+ saved_game.read_chunk(
+@@ -7378,7 +7378,7 @@ void CQuake3GameInterface::VariableLoadStrings( int type, varString_m &fmap )
+
+ if (idSize < 0 || static_cast<size_t>(idSize) >= sizeof(tempBuffer))
+ {
+- idSize = 0;
++ ::G_Error("invalid length for SIDS string in save game: %d bytes\n", idSize);
+ }
+
+ saved_game.read_chunk(
+@@ -7394,7 +7394,7 @@ void CQuake3GameInterface::VariableLoadStrings( int type, varString_m &fmap )
+
+ if (idSize < 0 || static_cast<size_t>(idSize) >= sizeof(tempBuffer2))
+ {
+- idSize = 0;
++ ::G_Error("invalid length for SVAL string in save game: %d bytes\n", idSize);
+ }
+
+ saved_game.read_chunk(
+diff --git a/code/game/g_roff.cpp b/code/game/g_roff.cpp
+index 9ec25adf..a337b284 100644
+--- a/code/game/g_roff.cpp
++++ b/code/game/g_roff.cpp
+@@ -704,7 +704,9 @@ void G_LoadCachedRoffs()
+ len);
+
+ if (len < 0 || static_cast<size_t>(len) >= sizeof(buffer))
+- len = 0;
++ {
++ ::G_Error("invalid length for RSTR string in save game: %d bytes\n", len);
++ }
+
+ saved_game.read_chunk(
+ INT_ID('R', 'S', 'T', 'R'),
+diff --git a/codeJK2/game/Q3_Registers.cpp b/codeJK2/game/Q3_Registers.cpp
+index 25c99cd1..f278c090 100644
+--- a/codeJK2/game/Q3_Registers.cpp
++++ b/codeJK2/game/Q3_Registers.cpp
+@@ -410,7 +410,7 @@ void Q3_VariableLoadFloats( varFloat_m &fmap )
+
+ if (idSize < 0 || static_cast<size_t>(idSize) >= sizeof(tempBuffer))
+ {
+- idSize = 0;
++ ::G_Error("invalid length for FIDS string in save game: %d bytes\n", idSize);
+ }
+
+ saved_game.read_chunk(
+@@ -460,7 +460,7 @@ void Q3_VariableLoadStrings( int type, varString_m &fmap )
+
+ if (idSize < 0 || static_cast<size_t>(idSize) >= sizeof(tempBuffer))
+ {
+- idSize = 0;
++ ::G_Error("invalid length for SIDS string in save game: %d bytes\n", idSize);
+ }
+
+ saved_game.read_chunk(
+@@ -476,7 +476,7 @@ void Q3_VariableLoadStrings( int type, varString_m &fmap )
+
+ if (idSize < 0 || static_cast<size_t>(idSize) >= sizeof(tempBuffer2))
+ {
+- idSize = 0;
++ ::G_Error("invalid length for SVAL string in save game: %d bytes\n", idSize);
+ }
+
+ saved_game.read_chunk(
+diff --git a/codeJK2/game/g_roff.cpp b/codeJK2/game/g_roff.cpp
+index b5c0240f..b1c9539e 100644
+--- a/codeJK2/game/g_roff.cpp
++++ b/codeJK2/game/g_roff.cpp
+@@ -680,7 +680,7 @@ void G_LoadCachedRoffs()
+
+ if (len < 0 || static_cast<size_t>(len) >= sizeof(buffer))
+ {
+- len = 0;
++ ::G_Error("invalid length for RSTR string in save game: %d bytes\n", len);
+ }
+
+ saved_game.read_chunk(
diff --git a/debian/patches/icarus-Fail-to-load-if-buffer-would-be-overflowed-don-t-j.patch b/debian/patches/icarus-Fail-to-load-if-buffer-would-be-overflowed-don-t-j.patch
new file mode 100644
index 0000000..cb0b479
--- /dev/null
+++ b/debian/patches/icarus-Fail-to-load-if-buffer-would-be-overflowed-don-t-j.patch
@@ -0,0 +1,67 @@
+From: Simon McVittie <smcv at debian.org>
+Date: Sat, 21 Jan 2017 15:10:39 +0000
+Subject: icarus: Fail to load if buffer would be overflowed, don't just skip
+
+Unfortunately CIcarus::BufferRead returns void, so we can't
+return a failure state from here, but we can at least print an
+error message.
+
+Signed-off-by: Simon McVittie <smcv at debian.org>
+Forwarded: https://github.com/JACoders/OpenJK/pull/905
+---
+ code/icarus/IcarusImplementation.cpp | 31 +++++++++++++++----------------
+ 1 file changed, 15 insertions(+), 16 deletions(-)
+
+diff --git a/code/icarus/IcarusImplementation.cpp b/code/icarus/IcarusImplementation.cpp
+index cc893331..c4e4f1fc 100644
+--- a/code/icarus/IcarusImplementation.cpp
++++ b/code/icarus/IcarusImplementation.cpp
+@@ -720,16 +720,16 @@ int CIcarus::Load()
+
+ if (sg_buffer_size < 0 || static_cast<size_t>(sg_buffer_size) > MAX_BUFFER_SIZE)
+ {
+- sg_buffer_size = 0;
+- }
+- else
+- {
+- std::uninitialized_copy_n(
+- sg_buffer_data,
+- sg_buffer_size,
+- m_byBuffer);
++ DestroyBuffer();
++ game->DebugPrint( IGameInterface::WL_ERROR, "invalid ISEQ length: %d bytes\n", sg_buffer_size);
++ return false;
+ }
+
++ std::uninitialized_copy_n(
++ sg_buffer_data,
++ sg_buffer_size,
++ m_byBuffer);
++
+ //Load all signals
+ if ( LoadSignals() == false )
+ {
+@@ -860,16 +860,15 @@ void CIcarus::BufferRead( void *pDstBuff, unsigned long ulNumBytesToRead )
+
+ if (sg_buffer_size < 0 || static_cast<size_t>(sg_buffer_size) > MAX_BUFFER_SIZE)
+ {
+- sg_buffer_size = 0;
+- }
+- else
+- {
+- std::uninitialized_copy_n(
+- sg_buffer_data,
+- sg_buffer_size,
+- m_byBuffer);
++ IGameInterface::GetGame()->DebugPrint( IGameInterface::WL_ERROR, "invalid ISEQ length: %d bytes\n", sg_buffer_size);
++ return;
+ }
+
++ std::uninitialized_copy_n(
++ sg_buffer_data,
++ sg_buffer_size,
++ m_byBuffer);
++
+ m_ulBytesRead = 0; //reset buffer
+ }
+
diff --git a/debian/patches/series b/debian/patches/series
index 858ce4d..af92b1f 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1,2 +1,5 @@
+Icarus-fix-bounds-check-when-loading-from-savegames.patch
+icarus-Fail-to-load-if-buffer-would-be-overflowed-don-t-j.patch
+game-If-strings-are-too-long-raise-error-instead-of-assum.patch
Pick-up-date-from-SOURCE_DATE_EPOCH-for-reproducible-buil.patch
g_utils-disarm-debug-code-that-writes-to-c-nofreeent.patch
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-games/openjk.git
More information about the Pkg-games-commits
mailing list