diff --git a/test/tinyobj_internal_tests.c b/test/tinyobj_internal_tests.c index aa89ddc..42475d1 100644 --- a/test/tinyobj_internal_tests.c +++ b/test/tinyobj_internal_tests.c @@ -985,6 +985,52 @@ void test_hash_table_grow(void) destroy_hash_table(&table); } +void test_parseLine_face_overflow(void) +{ + /* Regression for the stack-buffer-overflow fixed in parseLine(): an "f" + * statement that lists far more than TINYOBJ_MAX_FACES_PER_F_LINE vertices + * used to write past the fixed-size stack array f[] / command->f[]. The + * parser must cap the vertices it keeps and must never write out of bounds + * (the latter is what AddressSanitizer catches on the pre-fix code). */ + const char * line = + "f 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 " + "21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40"; + + { + /* Untriangulated: kept vertex count must stay within the array. */ + Command command; + int ret = parseLine(&command, line, strlen(line), /* triangulate */ 0); + TEST_CHECK(ret == 1); + TEST_CHECK(command.type == COMMAND_F); + TEST_CHECK(command.num_f <= TINYOBJ_MAX_FACES_PER_F_LINE); + } + + { + /* Triangulated: the 3*(N-2) emitted indices must also stay within + * command->f[] / command->f_num_verts[]. */ + Command command; + int ret = parseLine(&command, line, strlen(line), /* triangulate */ 1); + TEST_CHECK(ret == 1); + TEST_CHECK(command.type == COMMAND_F); + TEST_CHECK(command.num_f <= TINYOBJ_MAX_FACES_PER_F_LINE); + TEST_CHECK(command.num_f_num_verts <= TINYOBJ_MAX_FACES_PER_F_LINE); + } +} + +void test_parseLine_line_overflow(void) +{ + /* Regression: an "l" (polyline) statement with more than two vertices used + * to overflow the size-2 stack array f[] in parseLine()'s line branch + * (e.g. "l 1 2 3" wrote f[2]). Only the first two vertices are kept and no + * out-of-bounds write may occur. */ + const char * line = "l 1 2 3 4 5 6 7 8 9 10"; + Command command; + int ret = parseLine(&command, line, strlen(line), /* triangulate */ 0); + TEST_CHECK(ret == 1); + TEST_CHECK(command.type == COMMAND_F); + TEST_CHECK(command.num_f == 2); +} + TEST_LIST = { { "skip_space", test_skip_space }, { "skip_space_and_cr", test_skip_space_and_cr }, @@ -1007,5 +1053,7 @@ TEST_LIST = { { "hash_table_exists", test_hash_table_exists }, { "hash_table_get", test_hash_table_get }, { "hash_table_grow", test_hash_table_grow }, + { "parseLine_face_overflow", test_parseLine_face_overflow }, + { "parseLine_line_overflow", test_parseLine_line_overflow }, { 0 } // required by acutest }; diff --git a/tinyobj_loader_c.h b/tinyobj_loader_c.h index c5d307f..9884d56 100644 --- a/tinyobj_loader_c.h +++ b/tinyobj_loader_c.h @@ -1214,8 +1214,15 @@ static int parseLine(Command *command, const char *p, size_t p_len, tinyobj_vertex_index_t vi = parseRawTriple(&token); skip_space_and_cr(&token); - f[num_f] = vi; - num_f++; + /* Bounds-check: an OBJ "l" (line) statement may list more than two + * vertices, but this branch only keeps the first two (f[] has size 2, + * see the assert below). Without this guard a polyline such as + * "l 1 2 3" overflows the fixed-size stack array f[] (the assert is + * compiled out under NDEBUG). Extra vertices are dropped. */ + if (num_f < 2) { + f[num_f] = vi; + num_f++; + } } assert(num_f == 2); @@ -1241,8 +1248,14 @@ static int parseLine(Command *command, const char *p, size_t p_len, tinyobj_vertex_index_t vi = parseRawTriple(&token); skip_space_and_cr(&token); - f[num_f] = vi; - num_f++; + /* Bounds-check: an OBJ face/line may list more than + * TINYOBJ_MAX_FACES_PER_F_LINE vertices; without this guard the write + * overflows the fixed-size stack array f[] (the assert below is compiled + * out under NDEBUG). Extra vertices are dropped to preserve memory safety. */ + if (num_f < TINYOBJ_MAX_FACES_PER_F_LINE) { + f[num_f] = vi; + num_f++; + } } command->type = COMMAND_F; @@ -1255,9 +1268,16 @@ static int parseLine(Command *command, const char *p, size_t p_len, tinyobj_vertex_index_t i1; tinyobj_vertex_index_t i2 = f[1]; - assert(3 * num_f < TINYOBJ_MAX_FACES_PER_F_LINE); + /* Note: the triangulated output (3*(num_f-2) indices) may exceed + * command->f[]; the per-iteration guard below enforces the real bound, + * so we no longer assert that the whole n-gon fits. */ for (k = 2; k < num_f; k++) { + /* Bounds-check the triangulated output: an N-gon produces 3*(N-2) + * indices, which can exceed the fixed-size command->f[]. */ + if (3 * n + 2 >= TINYOBJ_MAX_FACES_PER_F_LINE) { + break; + } i1 = i2; i2 = f[k]; command->f[3 * n + 0] = i0; @@ -1272,7 +1292,7 @@ static int parseLine(Command *command, const char *p, size_t p_len, } else { size_t k = 0; - assert(num_f < TINYOBJ_MAX_FACES_PER_F_LINE); + assert(num_f <= TINYOBJ_MAX_FACES_PER_F_LINE); for (k = 0; k < num_f; k++) { command->f[k] = f[k]; }