Skip to content

Commit

Permalink
Merge #6084
Browse files Browse the repository at this point in the history
6084: Fix heap-buffer-overflow in DDNetLaser prediction code r=heinrich5991 a=Zwelf

Found while playing the Exit when doing the part at x:35 y:219. Verified that with the fix applied the crash doesn't happen anymore.

asan output:

```
=================================================================
==10996==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62000001de20 at pc 0x5555575208d0 bp 0x7fffffff4710 sp 0x7fffffff4708
READ of size 4 at 0x62000001de20 thread T0
[Detaching after fork from child process 11277]
    #0 0x5555575208cf in ExtractLaserInfoDDNet(CNetObj_DDNetLaser const*, CGameWorld*) /home/user/.local/bin/ddnet/src/src/game/client/laser_data.cpp:36:27
    #1 0x5555575af9c8 in CGameWorld::NetObjAdd(int, int, void const*, CNetObj_EntityEx const*) /home/user/.local/bin/ddnet/src/src/game/client/prediction/gameworld.cpp:493:11
    #2 0x5555574bf201 in CGameClient::UpdatePrediction() /home/user/.local/bin/ddnet/src/src/game/client/gameclient.cpp:2452:15
    #3 0x5555574aad89 in CGameClient::OnNewSnapshot() /home/user/.local/bin/ddnet/src/src/game/client/gameclient.cpp:1729:3
    #4 0x5555569562c7 in CClient::Update() /home/user/.local/bin/ddnet/src/src/engine/client/client.cpp:2751:22
    #5 0x55555696e4bd in CClient::Run() /home/user/.local/bin/ddnet/src/src/engine/client/client.cpp:3260:4
    #6 0x5555569caa8b in main /home/user/.local/bin/ddnet/src/src/engine/client/client.cpp:4753:11
    #7 0x7ffff4ea9d09 in __libc_start_main csu/../csu/libc-start.c:308:16
    #8 0x5555560f55c9 in _start (/home/user/.local/bin/ddnet/build-fast/DDNet+0xba15c9) (BuildId: 6d1b5aed4fc199ba75cdc083de5ada540ca4612b)

0x62000001de20 is located 0 bytes after 3488-byte region [0x62000001d080,0x62000001de20)
allocated by thread T0 here:
    #0 0x55555618e36e in __interceptor_malloc (/home/user/.local/bin/ddnet/build-fast/DDNet+0xc3a36e) (BuildId: 6d1b5aed4fc199ba75cdc083de5ada540ca4612b)
    #1 0x555556382591 in CSnapshotStorage::Add(int, long, int, void*, int, void*) /home/user/.local/bin/ddnet/src/src/engine/shared/snapshot.cpp:518:32
    #2 0x55555693aa6e in CClient::ProcessServerPacket(CNetChunk*, int, bool) /home/user/.local/bin/ddnet/src/src/engine/client/client.cpp:2088:31
    #3 0x55555694b48c in CClient::PumpNetwork() /home/user/.local/bin/ddnet/src/src/engine/client/client.cpp:2556:4
    #4 0x55555695ca61 in CClient::Update() /home/user/.local/bin/ddnet/src/src/engine/client/client.cpp:2868:2
    #5 0x55555696e4bd in CClient::Run() /home/user/.local/bin/ddnet/src/src/engine/client/client.cpp:3260:4
    #6 0x5555569caa8b in main /home/user/.local/bin/ddnet/src/src/engine/client/client.cpp:4753:11
    #7 0x7ffff4ea9d09 in __libc_start_main csu/../csu/libc-start.c:308:16

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/user/.local/bin/ddnet/src/src/game/client/laser_data.cpp:36:27 in ExtractLaserInfoDDNet(CNetObj_DDNetLaser const*, CGameWorld*)
Shadow bytes around the buggy address:
  0x62000001db80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x62000001dc00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x62000001dc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x62000001dd00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x62000001dd80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x62000001de00: 00 00 00 00[fa]fa fa fa fa fa fa fa fa fa fa fa
  0x62000001de80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x62000001df00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x62000001df80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x62000001e000: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x62000001e080: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==10996==ABORTING
```

<!-- 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
- [x] Considered possible null pointers and out of bounds array indexing
- [x] 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: Zwelf <zwelf@strct.cc>
  • Loading branch information
bors[bot] and Zwelf authored Dec 4, 2022
2 parents 8d17670 + 2268c08 commit a7408e0
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/game/client/prediction/gameworld.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ void CGameWorld::NetObjAdd(int ObjID, int ObjType, const void *pObjData, const C
else if((ObjType == NETOBJTYPE_LASER || ObjType == NETOBJTYPE_DDNETLASER) && m_WorldConfig.m_PredictWeapons)
{
CLaserData Data;
if(ObjType == NETOBJTYPE_PROJECTILE)
if(ObjType == NETOBJTYPE_LASER)
{
Data = ExtractLaserInfo((const CNetObj_Laser *)pObjData, this);
}
Expand Down

0 comments on commit a7408e0

Please sign in to comment.