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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant