diff --git a/src/draco/io/obj_decoder.cc b/src/draco/io/obj_decoder.cc index c9cbaadc..db8385fb 100644 --- a/src/draco/io/obj_decoder.cc +++ b/src/draco/io/obj_decoder.cc @@ -434,15 +434,34 @@ bool ObjDecoder::ParseFace(Status *status) { // Parse face indices. int num_valid_indices = 0; for (int i = 0; i < kMaxCorners; ++i) { - if (!ParseVertexIndices(&indices[i])) { + parser::SkipCharacters(buffer(), " \t"); + char ch; + if (!buffer()->Peek(&ch) || ch == '\n' || ch == '\r' || ch == '#') { if (i >= 3) { - break; // It's OK if there is no fourth or higher vertex index. + break; } *status = Status(Status::DRACO_ERROR, "Failed to parse vertex indices"); return true; } + if (!ParseVertexIndices(&indices[i])) { + *status = Status(Status::DRACO_ERROR, "Failed to parse vertex indices"); + return true; + } ++num_valid_indices; } + for (int i = 0; i < num_valid_indices; ++i) { + if (indices[i][0] > num_positions_ || + (indices[i][0] < 0 && num_positions_ + indices[i][0] < 0) || + (tex_att_id_ >= 0 && + (indices[i][1] > num_tex_coords_ || + (indices[i][1] < 0 && num_tex_coords_ + indices[i][1] < 0))) || + (norm_att_id_ >= 0 && + (indices[i][2] > num_normals_ || + (indices[i][2] < 0 && num_normals_ + indices[i][2] < 0)))) { + *status = Status(Status::DRACO_ERROR, "Face index out of range"); + return true; + } + } // Split quads and other n-gons into n - 2 triangles. const int nt = num_valid_indices - 2; // Iterate over triangles. @@ -646,6 +665,20 @@ void ObjDecoder::MapPointToVertexIndices( if (pos_att_id_ < 0) { return; } + if (indices[0] == 0 || indices[0] > num_positions_ || + (indices[0] < 0 && num_positions_ + indices[0] < 0)) { + return; + } + if (tex_att_id_ >= 0 && + (indices[1] > num_tex_coords_ || + (indices[1] < 0 && num_tex_coords_ + indices[1] < 0))) { + return; + } + if (norm_att_id_ >= 0 && + (indices[2] > num_normals_ || + (indices[2] < 0 && num_normals_ + indices[2] < 0))) { + return; + } if (indices[0] > 0) { out_point_cloud_->attribute(pos_att_id_) ->SetPointMapEntry(vert_id, AttributeValueIndex(indices[0] - 1)); @@ -663,9 +696,12 @@ void ObjDecoder::MapPointToVertexIndices( out_point_cloud_->attribute(tex_att_id_) ->SetPointMapEntry(vert_id, AttributeValueIndex(indices[1] - 1)); } else if (indices[1] < 0) { + const int32_t resolved = num_tex_coords_ + indices[1]; + if (resolved < 0) { + return; + } out_point_cloud_->attribute(tex_att_id_) - ->SetPointMapEntry(vert_id, - AttributeValueIndex(num_tex_coords_ + indices[1])); + ->SetPointMapEntry(vert_id, AttributeValueIndex(resolved)); } else { // Texture index not provided but expected. Insert 0 entry as the // default value. @@ -679,9 +715,12 @@ void ObjDecoder::MapPointToVertexIndices( out_point_cloud_->attribute(norm_att_id_) ->SetPointMapEntry(vert_id, AttributeValueIndex(indices[2] - 1)); } else if (indices[2] < 0) { + const int32_t resolved = num_normals_ + indices[2]; + if (resolved < 0) { + return; + } out_point_cloud_->attribute(norm_att_id_) - ->SetPointMapEntry(vert_id, - AttributeValueIndex(num_normals_ + indices[2])); + ->SetPointMapEntry(vert_id, AttributeValueIndex(resolved)); } else { // Normal index not provided but expected. Insert 0 entry as the default // value. diff --git a/src/draco/io/obj_decoder_test.cc b/src/draco/io/obj_decoder_test.cc index a46a15a8..ff858e83 100644 --- a/src/draco/io/obj_decoder_test.cc +++ b/src/draco/io/obj_decoder_test.cc @@ -306,4 +306,223 @@ TEST_F(ObjDecoderTest, TestObjDecodingAll) { test_decoding("inf_nan.obj"); } +TEST_F(ObjDecoderTest, RejectOutOfRangeIndices) { + // Tests that OBJ with out-of-range indices are safely rejected (fixes #1194). + const auto decode_string = [](const std::string &obj) -> Status { + DecoderBuffer buffer; + buffer.Init(obj.data(), obj.size()); + ObjDecoder decoder; + Mesh mesh; + return decoder.DecodeFromBuffer(&buffer, &mesh); + }; + + // Texture coordinate index out of range (positive). + { + const std::string obj = + "v 0 0 0\n" + "v 1 0 0\n" + "v 0 1 0\n" + "vt 0 0\n" + "vt 0 0\n" + "vt 0 0\n" + "vn 0 0 1\n" + "vn 0 0 1\n" + "vn 0 0 1\n" + "f 1/100/1 2/100/2 3/100/3\n"; + const Status status = decode_string(obj); + EXPECT_FALSE(status.ok()); + EXPECT_EQ(status.code(), Status::DRACO_ERROR); + EXPECT_EQ(status.error_msg_string(), "Face index out of range"); + } + + // Vertex position index out of range (positive). + { + const std::string obj = + "v 0 0 0\n" + "v 1 0 0\n" + "v 0 1 0\n" + "f 1 4 2\n"; + const Status status = decode_string(obj); + EXPECT_FALSE(status.ok()); + EXPECT_EQ(status.code(), Status::DRACO_ERROR); + EXPECT_EQ(status.error_msg_string(), "Face index out of range"); + } + + // Vertex position index out of range (negative). + { + const std::string obj = + "v 0 0 0\n" + "v 1 0 0\n" + "v 0 1 0\n" + "f -1 -4 -2\n"; + const Status status = decode_string(obj); + EXPECT_FALSE(status.ok()); + EXPECT_EQ(status.code(), Status::DRACO_ERROR); + EXPECT_EQ(status.error_msg_string(), "Face index out of range"); + } + + // Normal index out of range (positive). + { + const std::string obj = + "v 0 0 0\n" + "v 1 0 0\n" + "v 0 1 0\n" + "vn 0 0 1\n" + "f 1//10 2//1 3//1\n"; + const Status status = decode_string(obj); + EXPECT_FALSE(status.ok()); + EXPECT_EQ(status.code(), Status::DRACO_ERROR); + EXPECT_EQ(status.error_msg_string(), "Face index out of range"); + } + + // Normal index out of range (negative). + { + const std::string obj = + "v 0 0 0\n" + "v 1 0 0\n" + "v 0 1 0\n" + "vn 0 0 1\n" + "f 1//-2 2//-1 3//-1\n"; + const Status status = decode_string(obj); + EXPECT_FALSE(status.ok()); + EXPECT_EQ(status.code(), Status::DRACO_ERROR); + EXPECT_EQ(status.error_msg_string(), "Face index out of range"); + } + + // Texture coordinate index out of range (negative). + { + const std::string obj = + "v 0 0 0\n" + "v 1 0 0\n" + "v 0 1 0\n" + "vt 0 0\n" + "f 1/-2 2/-1 3/-1\n"; + const Status status = decode_string(obj); + EXPECT_FALSE(status.ok()); + EXPECT_EQ(status.code(), Status::DRACO_ERROR); + EXPECT_EQ(status.error_msg_string(), "Face index out of range"); + } + + // Point cloud decoding with out-of-range texture index. + { + const std::string obj = + "v 0 0 0\n" + "v 1 0 0\n" + "v 0 1 0\n" + "vt 0 0\n" + "f 1/100 2/100 3/100\n"; + DecoderBuffer buffer; + buffer.Init(obj.data(), obj.size()); + ObjDecoder decoder; + PointCloud pc; + const Status status = decoder.DecodeFromBuffer(&buffer, &pc); + EXPECT_FALSE(status.ok()); + EXPECT_EQ(status.code(), Status::DRACO_ERROR); + EXPECT_EQ(status.error_msg_string(), "Face index out of range"); + } + + // Quad face where 4th corner has out-of-range index. + { + const std::string obj = + "v 0 0 0\n" + "v 1 0 0\n" + "v 1 1 0\n" + "v 0 1 0\n" + "f 1 2 3 99\n"; + const Status status = decode_string(obj); + EXPECT_FALSE(status.ok()); + EXPECT_EQ(status.code(), Status::DRACO_ERROR); + EXPECT_EQ(status.error_msg_string(), "Face index out of range"); + } + + // Quad face where 4th corner has out-of-range index (negative). + { + const std::string obj = + "v 0 0 0\n" + "v 1 0 0\n" + "v 1 1 0\n" + "v 0 1 0\n" + "f 1 2 3 -10\n"; + const Status status = decode_string(obj); + EXPECT_FALSE(status.ok()); + EXPECT_EQ(status.code(), Status::DRACO_ERROR); + EXPECT_EQ(status.error_msg_string(), "Face index out of range"); + } + + // Quad face where 4th corner has out-of-range texture index. + { + const std::string obj = + "v 0 0 0\n" + "v 1 0 0\n" + "v 1 1 0\n" + "v 0 1 0\n" + "vt 0 0\n" + "f 1/1 2/1 3/1 4/99\n"; + const Status status = decode_string(obj); + EXPECT_FALSE(status.ok()); + EXPECT_EQ(status.code(), Status::DRACO_ERROR); + EXPECT_EQ(status.error_msg_string(), "Face index out of range"); + } + + // Quad face where 4th corner has invalid texture index 0. + { + const std::string obj = + "v 0 0 0\n" + "v 1 0 0\n" + "v 1 1 0\n" + "v 0 1 0\n" + "vt 0 0\n" + "f 1/1 2/1 3/1 4/0\n"; + const Status status = decode_string(obj); + EXPECT_FALSE(status.ok()); + } + + // Quad face where 4th corner is index 0. + { + const std::string obj = + "v 0 0 0\n" + "v 1 0 0\n" + "v 1 1 0\n" + "v 0 1 0\n" + "f 1 2 3 0\n"; + const Status status = decode_string(obj); + EXPECT_FALSE(status.ok()); + } + + // Point cloud decoding with out-of-range position index. + { + const std::string obj = + "v 0 0 0\n" + "v 1 0 0\n" + "v 0 1 0\n" + "f 100 2 3\n"; + DecoderBuffer buffer; + buffer.Init(obj.data(), obj.size()); + ObjDecoder decoder; + PointCloud pc; + const Status status = decoder.DecodeFromBuffer(&buffer, &pc); + EXPECT_FALSE(status.ok()); + EXPECT_EQ(status.code(), Status::DRACO_ERROR); + EXPECT_EQ(status.error_msg_string(), "Face index out of range"); + } + + // Point cloud decoding with out-of-range normal index. + { + const std::string obj = + "v 0 0 0\n" + "v 1 0 0\n" + "v 0 1 0\n" + "vn 0 0 1\n" + "f 1//100 2//1 3//1\n"; + DecoderBuffer buffer; + buffer.Init(obj.data(), obj.size()); + ObjDecoder decoder; + PointCloud pc; + const Status status = decoder.DecodeFromBuffer(&buffer, &pc); + EXPECT_FALSE(status.ok()); + EXPECT_EQ(status.code(), Status::DRACO_ERROR); + EXPECT_EQ(status.error_msg_string(), "Face index out of range"); + } +} + } // namespace draco