6214: Fix undefined behavior when unpacking snapshot deltas r=def- a=Robyt3

From #5646.

## Checklist

- [X] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [X] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)


Co-authored-by: Robert Müller <robytemueller@gmail.com>
This commit is contained in:
bors[bot] 2022-12-31 12:01:05 +00:00 committed by GitHub
commit 8242238193
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 14 additions and 4 deletions

View file

@ -7,7 +7,9 @@
#include <climits>
#include <cstdlib>
#include <base/math.h>
#include <base/system.h>
#include <game/generated/protocolglue.h>
// 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<long>(OutValue, std::numeric_limits<int>::min(), std::numeric_limits<int>::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
{
@ -681,6 +689,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;

View file

@ -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);