From d63a7a3c844392eea19810e0ecd0b767febfdfb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Fri, 30 Dec 2022 13:54:37 +0100 Subject: [PATCH 1/2] Fix left shift of negative value when unpacking snapshot delta ``` src/engine/shared/snapshot.cpp:693:28: runtime error: left shift of negative value -1 0 0x55cae1608071 in CSnapshotBuilder::NewItem(int, int, int) src/engine/shared/snapshot.cpp:686 1 0x55cae1603fe0 in CSnapshotDelta::UnpackDelta(CSnapshot*, CSnapshot*, void const*, int) src/engine/shared/snapshot.cpp:390 2 0x55cae15544c6 in CDemoPlayer::DoTick() src/engine/shared/demo.cpp:624 3 0x55cae1566c27 in CDemoPlayer::Update(bool) src/engine/shared/demo.cpp:1016 4 0x55cae03c07fa in CClient::Update() src/engine/client/client.cpp:2628 5 0x55cae03e5cfd in CClient::Run() src/engine/client/client.cpp:3220 6 0x55cae0463413 in main src/engine/client/client.cpp:4717 7 0x7fbc7d855d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 8 0x7fbc7d855e3f in __libc_start_main_impl ../csu/libc-start.c:392 9 0x55cadff2aa14 in _start (build-demofuzz/DDNet+0x2493a14) ``` --- src/engine/shared/snapshot.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/engine/shared/snapshot.cpp b/src/engine/shared/snapshot.cpp index cd97c9a71..fb01a178d 100644 --- a/src/engine/shared/snapshot.cpp +++ b/src/engine/shared/snapshot.cpp @@ -681,6 +681,8 @@ void *CSnapshotBuilder::NewItem(int Type, int ID, int Size) if(Type < 0) return pObj; } + else if(Type < 0) + return nullptr; mem_zero(pObj, sizeof(CSnapshotItem) + Size); pObj->m_TypeAndID = (Type << 16) | ID; From 4118074768fef5912c5241a8f6172f33297d30cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Fri, 30 Dec 2022 13:57:57 +0100 Subject: [PATCH 2/2] Fix signed integer overflow when unpacking snapshot delta ``` src/engine/shared/snapshot.cpp:219:18: runtime error: signed integer overflow: -2011501152 + -1594687485 cannot be represented in type 'int' 0 0x5593cbc3534c in CSnapshotDelta::UndiffItem(int const*, int*, int*, int, int*) src/engine/shared/snapshot.cpp:219 1 0x5593cbc3852d in CSnapshotDelta::UnpackDelta(CSnapshot*, CSnapshot*, void const*, int) src/engine/shared/snapshot.cpp:442 2 0x5593cbb881a6 in CDemoPlayer::DoTick() src/engine/shared/demo.cpp:624 3 0x5593cbb9a907 in CDemoPlayer::Update(bool) src/engine/shared/demo.cpp:1016 4 0x5593ca9f44da in CClient::Update() src/engine/client/client.cpp:2628 5 0x5593caa199dd in CClient::Run() src/engine/client/client.cpp:3220 6 0x5593caa970f3 in main src/engine/client/client.cpp:4717 7 0x7fe086d04d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 8 0x7fe086d04e3f in __libc_start_main_impl ../csu/libc-start.c:392 9 0x5593ca55e6f4 in _start (build-demofuzz/DDNet+0x24936f4) ``` --- src/engine/shared/snapshot.cpp | 14 +++++++++++--- src/engine/shared/snapshot.h | 2 +- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/engine/shared/snapshot.cpp b/src/engine/shared/snapshot.cpp index fb01a178d..7f3f8ee0c 100644 --- a/src/engine/shared/snapshot.cpp +++ b/src/engine/shared/snapshot.cpp @@ -7,7 +7,9 @@ #include #include +#include #include + #include // CSnapshot @@ -212,11 +214,14 @@ int CSnapshotDelta::DiffItem(const int *pPast, const int *pCurrent, int *pOut, i return Needed; } -void CSnapshotDelta::UndiffItem(const int *pPast, int *pDiff, int *pOut, int Size, int *pDataRate) +bool CSnapshotDelta::UndiffItem(const int *pPast, int *pDiff, int *pOut, int Size, int *pDataRate) { while(Size) { - *pOut = *pPast + *pDiff; + const long OutValue = (long)*pPast + *pDiff; + if(!in_range(OutValue, std::numeric_limits::min(), std::numeric_limits::max())) + return false; + *pOut = (int)OutValue; if(*pDiff == 0) *pDataRate += 1; @@ -232,6 +237,8 @@ void CSnapshotDelta::UndiffItem(const int *pPast, int *pDiff, int *pOut, int Siz pDiff++; Size--; } + + return true; } CSnapshotDelta::CSnapshotDelta() @@ -439,7 +446,8 @@ int CSnapshotDelta::UnpackDelta(CSnapshot *pFrom, CSnapshot *pTo, const void *pS if(FromIndex != -1) { // we got an update so we need to apply the diff - UndiffItem(pFrom->GetItem(FromIndex)->Data(), pData, pNewData, ItemSize / 4, &m_aSnapshotDataRate[Type]); + if(!UndiffItem(pFrom->GetItem(FromIndex)->Data(), pData, pNewData, ItemSize / 4, &m_aSnapshotDataRate[Type])) + return -3; } else // no previous, just copy the pData { diff --git a/src/engine/shared/snapshot.h b/src/engine/shared/snapshot.h index bb126a136..77261b387 100644 --- a/src/engine/shared/snapshot.h +++ b/src/engine/shared/snapshot.h @@ -88,7 +88,7 @@ private: int m_aSnapshotDataUpdates[CSnapshot::MAX_TYPE + 1]; CData m_Empty; - static void UndiffItem(const int *pPast, int *pDiff, int *pOut, int Size, int *pDataRate); + static bool UndiffItem(const int *pPast, int *pDiff, int *pOut, int Size, int *pDataRate); public: static int DiffItem(const int *pPast, const int *pCurrent, int *pOut, int Size);