edg-l/ddnet
1
0
Fork 0
mirror of https://github.com/ddnet/ddnet.git synced 2024-09-20 09:34:19 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge #6678

Browse source 
6678: Fix undefined behavior in `CSnapshotDelta::DiffItem` r=Chairn a=Robyt3

Cast `int`s to `unsigned` before subtracting to ensure that integer wrapping is being used instead of causing undefined behavior. Same as in `UndiffItem`.

```
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/master/src/base/math.h:16:40 in
src/master/src/engine/shared/snapshot.cpp:206:21: runtime error: signed integer overflow: 256 - -2147483648 cannot be represented in type 'int'
    0 0x7650b7 in CSnapshotDelta::DiffItem(int const*, int const*, int*, int) src/master/src/engine/shared/snapshot.cpp:206:21
    1 0x765cea in CSnapshotDelta::CreateDelta(CSnapshot*, CSnapshot*, void*) src/master/src/engine/shared/snapshot.cpp:323:7
    2 0x51a0e2 in CServer::DoSnapshot() src/master/src/engine/server/server.cpp:964:36
    3 0x537486 in CServer::Run() src/master/src/engine/server/server.cpp:2818:6
    4 0x4feeb7 in main src/master/src/engine/server/main.cpp:190:21
    5 0x7fc51ec27d09 in __libc_start_main csu/../csu/libc-start.c:308:16
    6 0x4c3819 in _start (servers/DDNet-Server-ubsan+0x4c3819)

src/master/src/engine/shared/snapshot.cpp:206:21: runtime error: signed integer overflow: 1645289600 - -2139062144 cannot be represented in type 'int'
    0 0x7650b7 in CSnapshotDelta::DiffItem(int const*, int const*, int*, int) src/master/src/engine/shared/snapshot.cpp:206:21
    1 0x765cea in CSnapshotDelta::CreateDelta(CSnapshot*, CSnapshot*, void*) src/master/src/engine/shared/snapshot.cpp:323:7
    2 0x51a0e2 in CServer::DoSnapshot() src/master/src/engine/server/server.cpp:964:36
    3 0x537486 in CServer::Run() src/master/src/engine/server/server.cpp:2818:6
    4 0x4feeb7 in main src/master/src/engine/server/main.cpp:190:21
    5 0x7efd50c4ed09 in __libc_start_main csu/../csu/libc-start.c:308:16
    6 0x4c3819 in _start (servers/DDNet-Server-ubsan+0x4c3819)
```

See #6650.

<!-- What is the motivation for the changes of this pull request? -->

<!-- Note that builds and other checks will be run for your change. Don't feel intimidated by failures in some of the checks. If you can't resolve them yourself, experienced devs can also resolve them before merging your pull request. -->

## 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
- [ ] 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] 2023-05-27 19:29:12 +00:00 committed by GitHub
parent 7cdc92859e 36dd5c5075
commit 9dfcbd9ff4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
1 changed files with 2 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
src/engine/shared/snapshot.cpp
View file

@ -203,7 +203,8 @@ int CSnapshotDelta::DiffItem(const int *pPast, const int *pCurrent, int *pOut, i
int Needed = 0;
while(Size)
{
*pOut = *pCurrent - *pPast;
// subtraction with wrapping by casting to unsigned
*pOut = (unsigned)*pCurrent - (unsigned)*pPast;
Needed |= *pOut;
pOut++;
pPast++;