Skip to content

Fix invalid pointer dereference in OBJ decoder for faces without vertices - #1204

Open
gn00295120 wants to merge 1 commit into
google:mainfrom
gn00295120:fix/obj-invalid-deref
Open

Fix invalid pointer dereference in OBJ decoder for faces without vertices#1204
gn00295120 wants to merge 1 commit into
google:mainfrom
gn00295120:fix/obj-invalid-deref

Conversation

@gn00295120

Copy link
Copy Markdown

Summary

ObjDecoder crashes when an OBJ file contains face definitions (f 1 2 3) but no preceding vertex positions (v ...). The position attribute ID stays at its initial value of -1, and PointCloud::attribute(-1) dereferences out of bounds.

Additionally, negative OBJ vertex indices (e.g., f -1 -2 -3) are resolved against num_positions_ without checking for underflow.

Changes

  • After the counting pass, reject files that have faces but zero vertex positions (early error instead of UB)
  • Guard MapPointToVertexIndices against pos_att_id_ < 0
  • Validate that resolved negative indices are non-negative before use

Reproduction

# Build with ASAN
cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ \
  -DCMAKE_CXX_FLAGS="-fsanitize=address -fno-omit-frame-pointer" \
  -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address" .
cmake --build build

# 8-byte OBJ file triggers SEGV
printf 'f 1 2 3\n' > crash.obj
./build/draco_encoder -i crash.obj

ASAN trace (before fix):

==PID==ERROR: AddressSanitizer: SEGV on unknown address 0xfffffffffffffff8
    #0 in draco::PointCloud::attribute(int) point_cloud.h:89
    #1 in draco::ObjDecoder::MapPointToVertexIndices obj_decoder.cc:643
    #2 in draco::ObjDecoder::ParseFace obj_decoder.cc:450
…ices

ObjDecoder crashes when an OBJ file has face definitions but no
vertex positions. pos_att_id_ stays at -1, so attribute(-1) reads
out of bounds. Negative OBJ indices also aren't bounds-checked.

- Reject files with faces but zero positions after the counting pass
- Guard MapPointToVertexIndices against pos_att_id_ < 0
- Validate resolved negative indices are non-negative
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant