Skip to content

io: validate vertex, texture, and normal index bounds in obj decoder (fixes #1194) #1243

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jdymitarai wants to merge 1 commit into
base: main
Choose a base branch
from
Open

io: validate vertex, texture, and normal index bounds in obj decoder (fixes #1194) #1243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 45 additions & 6 deletions src/draco/io/obj_decoder.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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));
Expand All @@ -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.
Expand All @@ -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.
Expand Down
219 changes: 219 additions & 0 deletions src/draco/io/obj_decoder_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading