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 <[email protected]> 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 <[email protected]> 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 <[email protected]> +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 <[email protected]> +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 <[email protected]> +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 <[email protected]> +--- + 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 <[email protected]> +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 <[email protected]> +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 _______________________________________________ Pkg-games-commits mailing list [email protected] http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-games-commits
