From 1b4b1bedda410265324bdd14a2ada301413dba98 Mon Sep 17 00:00:00 2001 From: Syoyo Fujita Date: Sat, 9 May 2026 14:02:34 +0900 Subject: [PATCH] Harden v3 C parser against untrusted glTF input Threat model: parser is intended for server-side processing of attacker- supplied glTF/GLB. Two adversarial review rounds plus a 1-hour libFuzzer run (4 workers, ASan+UBSan, ~420M execs total, zero new artifacts) drove this set of fixes. Concrete PoCs in tests/v3/security/ confirmed each issue was exploitable on the prior code. Path traversal (CRITICAL): tg3__load_external_file concatenated base_dir with the JSON-supplied URI verbatim. A glTF with "uri":"../../../tmp/secret" successfully loaded the file from outside base_dir (verified by FNV64 match). New tg3__uri_is_safe rejects empty, NUL, leading / or \\, Windows drive prefixes, and any '..' segment. Path-buffer length checks switched to saturating subtraction so 32-bit size_t cannot wrap. Sign-coercion in byteStride: int32_t -1 was cast directly to uint32_t, producing 0xFFFFFFFF and propagating into downstream count*stride math. Restrict to glTF spec range: 0 (tightly packed) or [4, 252]. Index validation: parsed int32 index fields (accessor.bufferView, primitive.indices/material/attributes, node.mesh/skin/camera/light, scene.nodes[], skin.joints[], animation channel/sampler refs, MSFT_lod ids, KHR_audio emitter/source refs, etc.) were stored unchecked. New tg3__validate_indices walks every index field and returns TG3_ERR_INVALID_INDEX on out-of-range. Gated by tg3_parse_options.validate_indices, defaulting to 1. Use-after-free on parse failure (PRE-EXISTING, surfaced by ASan during fix verification): tg3_parse and tg3_parse_glb destroyed model->arena_ on error paths, but error messages on the user-facing tg3_error_stack were arena-allocated. Any caller reading errors.entries[i].message after parse failure read freed memory. tg3_model_free is now sole arena owner; arena lives across error paths so messages stay valid until the caller frees the model. Other fixes: - tg3_parse_glb: hoist tg3__model_init before header parse so callers can safely tg3_model_free on header failure. - tg3__parse_primitive morph targets: when arena alloc returns NULL, pair with target_counts[ti]=0 so validators do not deref. - Defensive 'if (!tarr) continue' in the morph-target validator loop. - New Security Considerations block in tiny_gltf_v3.h documents the threat model, default-on validation, fs-callback contract, and error message lifetime. Verification: 13 internal tests in tester_v3_c (incl. 7 new security regressions covering path traversal absolute and relative, fs-callback no-call assertion, byteStride wrap, OOB index, opt-in raw mode, ext fields, and arena-message lifetime), 134/134 Khronos sample models match v1 ground truth digest, 1-hour ASan+UBSan fuzz on the final code clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/tester_v3_c.c | 247 ++++++++++++++++ tests/v3/security/negstride.gltf | 8 + tests/v3/security/oob_buffer_view.gltf | 9 + tests/v3/security/oob_extension_indices.gltf | 19 ++ tests/v3/security/traversal_relative.gltf | 14 + tiny_gltf_v3.c | 278 ++++++++++++++++++- tiny_gltf_v3.h | 35 +++ 7 files changed, 596 insertions(+), 14 deletions(-) create mode 100644 tests/v3/security/negstride.gltf create mode 100644 tests/v3/security/oob_buffer_view.gltf create mode 100644 tests/v3/security/oob_extension_indices.gltf create mode 100644 tests/v3/security/traversal_relative.gltf diff --git a/tests/tester_v3_c.c b/tests/tester_v3_c.c index 7cb3efc..fe477e5 100644 --- a/tests/tester_v3_c.c +++ b/tests/tester_v3_c.c @@ -423,6 +423,232 @@ static int check_huge_integer_field_rejected(void) { return 1; } +/* ===== Security regression tests ============================================ */ + +/* fs read_file callback that records calls into *(int *)user_data and never + * succeeds — used to verify path-traversal URIs never reach the filesystem. */ +static int32_t recording_read_file(uint8_t **out_data, uint64_t *out_size, + const char *path, uint32_t path_len, + void *user_data) { + (void)out_data; (void)out_size; (void)path; (void)path_len; + if (user_data) *(int *)user_data += 1; + return 0; +} + +static int check_path_traversal_rejected(void) { + static const uint8_t json[] = + "{\"asset\":{\"version\":\"2.0\"}," + "\"buffers\":[{\"uri\":\"../../etc/passwd\",\"byteLength\":4}]}"; + tg3_model model; + tg3_error_stack errors; + tg3_parse_options opts; + tg3_error_code err; + int fs_calls = 0; + uint32_t i; + int saw = 0; + + tg3_error_stack_init(&errors); + tg3_parse_options_init(&opts); + opts.fs.read_file = recording_read_file; + opts.fs.user_data = &fs_calls; + err = tg3_parse(&model, &errors, json, (uint64_t)(sizeof(json) - 1), + "/some/base", 10, &opts); + if (err == TG3_OK) { + fprintf(stderr, "path traversal NOT rejected\n"); + goto fail; + } + if (fs_calls != 0) { + fprintf(stderr, "fs.read_file called %d times for traversal URI\n", fs_calls); + goto fail; + } + for (i = 0; i < errors.count; ++i) { + if (errors.entries[i].code == TG3_ERR_INVALID_VALUE) saw = 1; + } + if (!saw) { + fprintf(stderr, "expected TG3_ERR_INVALID_VALUE on traversal\n"); + goto fail; + } + tg3_model_free(&model); + tg3_error_stack_free(&errors); + return 1; +fail: + tg3_model_free(&model); + tg3_error_stack_free(&errors); + return 0; +} + +static int check_absolute_uri_rejected(void) { + static const uint8_t json[] = + "{\"asset\":{\"version\":\"2.0\"}," + "\"buffers\":[{\"uri\":\"/etc/passwd\",\"byteLength\":4}]}"; + tg3_model model; + tg3_error_stack errors; + tg3_parse_options opts; + tg3_error_code err; + int fs_calls = 0; + + tg3_error_stack_init(&errors); + tg3_parse_options_init(&opts); + opts.fs.read_file = recording_read_file; + opts.fs.user_data = &fs_calls; + err = tg3_parse(&model, &errors, json, (uint64_t)(sizeof(json) - 1), + "/base", 5, &opts); + if (err == TG3_OK || fs_calls != 0) { + fprintf(stderr, "absolute URI not rejected (rc=%d, fs_calls=%d)\n", + (int)err, fs_calls); + tg3_model_free(&model); + tg3_error_stack_free(&errors); + return 0; + } + tg3_model_free(&model); + tg3_error_stack_free(&errors); + return 1; +} + +static int check_negative_byte_stride_rejected(void) { + static const uint8_t json[] = + "{\"asset\":{\"version\":\"2.0\"}," + "\"buffers\":[{\"byteLength\":4}]," + "\"bufferViews\":[{\"buffer\":0,\"byteLength\":4,\"byteStride\":-1}]}"; + tg3_model model; + tg3_error_stack errors; + tg3_parse_options opts; + tg3_error_code err; + + tg3_error_stack_init(&errors); + tg3_parse_options_init(&opts); + err = tg3_parse(&model, &errors, json, (uint64_t)(sizeof(json) - 1), "", 0, &opts); + if (err == TG3_OK) { + fprintf(stderr, "negative byteStride NOT rejected\n"); + tg3_model_free(&model); + tg3_error_stack_free(&errors); + return 0; + } + tg3_model_free(&model); + tg3_error_stack_free(&errors); + return 1; +} + +static int check_oob_index_rejected(void) { + static const uint8_t json[] = + "{\"asset\":{\"version\":\"2.0\"}," + "\"buffers\":[{\"byteLength\":4}]," + "\"bufferViews\":[{\"buffer\":0,\"byteLength\":4}]," + "\"accessors\":[{\"bufferView\":1000000,\"componentType\":5121," + "\"count\":1,\"type\":\"SCALAR\"}]}"; + tg3_model model; + tg3_error_stack errors; + tg3_parse_options opts; + tg3_error_code err; + + tg3_error_stack_init(&errors); + tg3_parse_options_init(&opts); + err = tg3_parse(&model, &errors, json, (uint64_t)(sizeof(json) - 1), "", 0, &opts); + if (err != TG3_ERR_INVALID_INDEX) { + fprintf(stderr, "OOB index expected TG3_ERR_INVALID_INDEX, got %d\n", (int)err); + tg3_model_free(&model); + tg3_error_stack_free(&errors); + return 0; + } + tg3_model_free(&model); + tg3_error_stack_free(&errors); + return 1; +} + +static int check_oob_index_opt_in(void) { + /* When validate_indices=0, parse should accept the same out-of-range index. */ + static const uint8_t json[] = + "{\"asset\":{\"version\":\"2.0\"}," + "\"buffers\":[{\"byteLength\":4}]," + "\"bufferViews\":[{\"buffer\":0,\"byteLength\":4}]," + "\"accessors\":[{\"bufferView\":1000000,\"componentType\":5121," + "\"count\":1,\"type\":\"SCALAR\"}]}"; + tg3_model model; + tg3_error_stack errors; + tg3_parse_options opts; + tg3_error_code err; + + tg3_error_stack_init(&errors); + tg3_parse_options_init(&opts); + opts.validate_indices = 0; + err = tg3_parse(&model, &errors, json, (uint64_t)(sizeof(json) - 1), "", 0, &opts); + if (err != TG3_OK) { + fprintf(stderr, "validate_indices=0 should accept OOB index, got %d\n", (int)err); + tg3_model_free(&model); + tg3_error_stack_free(&errors); + return 0; + } + if (model.accessors_count != 1 || model.accessors[0].buffer_view != 1000000) { + fprintf(stderr, "OOB index not preserved when validation off\n"); + tg3_model_free(&model); + tg3_error_stack_free(&errors); + return 0; + } + tg3_model_free(&model); + tg3_error_stack_free(&errors); + return 1; +} + +static int check_extension_index_oob_rejected(void) { + /* MSFT_lod and KHR_audio index fields must be validated when + * validate_indices=1, otherwise downstream consumers can read OOB. */ + static const uint8_t json[] = + "{\"asset\":{\"version\":\"2.0\"}," + "\"nodes\":[{\"extensions\":{\"MSFT_lod\":{\"ids\":[99999]}}}]}"; + tg3_model model; + tg3_error_stack errors; + tg3_parse_options opts; + tg3_error_code err; + + tg3_error_stack_init(&errors); + tg3_parse_options_init(&opts); + err = tg3_parse(&model, &errors, json, (uint64_t)(sizeof(json) - 1), "", 0, &opts); + if (err != TG3_ERR_INVALID_INDEX) { + fprintf(stderr, "MSFT_lod OOB index expected TG3_ERR_INVALID_INDEX, got %d\n", (int)err); + tg3_model_free(&model); + tg3_error_stack_free(&errors); + return 0; + } + tg3_model_free(&model); + tg3_error_stack_free(&errors); + return 1; +} + +static int check_error_messages_survive_parse_failure(void) { + /* Regression: parse failure must not invalidate arena-allocated error + * message strings on the user's tg3_error_stack before model_free. */ + static const uint8_t json[] = + "{\"asset\":{\"version\":\"2.0\"}," + "\"buffers\":[{\"byteLength\":4}]," + "\"bufferViews\":[{\"buffer\":0,\"byteLength\":4,\"byteStride\":-1}]}"; + tg3_model model; + tg3_error_stack errors; + tg3_parse_options opts; + tg3_error_code err; + uint32_t i; + int seen_stride_msg = 0; + + tg3_error_stack_init(&errors); + tg3_parse_options_init(&opts); + err = tg3_parse(&model, &errors, json, (uint64_t)(sizeof(json) - 1), "", 0, &opts); + if (err == TG3_OK) goto fail; + for (i = 0; i < errors.count; ++i) { + const char *m = errors.entries[i].message; + if (m && strstr(m, "byteStride")) seen_stride_msg = 1; + } + if (!seen_stride_msg) { + fprintf(stderr, "byteStride error message missing or unreadable after parse\n"); + goto fail; + } + tg3_model_free(&model); + tg3_error_stack_free(&errors); + return 1; +fail: + tg3_model_free(&model); + tg3_error_stack_free(&errors); + return 0; +} + static int parse_file_arg(const char *path) { FILE *fp = fopen(path, "rb"); uint8_t *buf; @@ -521,5 +747,26 @@ int main(int argc, char **argv) { if (!check_huge_integer_field_rejected()) { return 1; } + if (!check_path_traversal_rejected()) { + return 1; + } + if (!check_absolute_uri_rejected()) { + return 1; + } + if (!check_negative_byte_stride_rejected()) { + return 1; + } + if (!check_oob_index_rejected()) { + return 1; + } + if (!check_oob_index_opt_in()) { + return 1; + } + if (!check_extension_index_oob_rejected()) { + return 1; + } + if (!check_error_messages_survive_parse_failure()) { + return 1; + } return 0; } diff --git a/tests/v3/security/negstride.gltf b/tests/v3/security/negstride.gltf new file mode 100644 index 0000000..3c264de --- /dev/null +++ b/tests/v3/security/negstride.gltf @@ -0,0 +1,8 @@ +{ + "asset": {"version": "2.0"}, + "buffers": [{"byteLength": 4}], + "bufferViews": [ + {"buffer": 0, "byteOffset": 0, "byteLength": 4, "byteStride": -1} + ], + "scenes": [{"nodes": []}] +} diff --git a/tests/v3/security/oob_buffer_view.gltf b/tests/v3/security/oob_buffer_view.gltf new file mode 100644 index 0000000..c82bb0f --- /dev/null +++ b/tests/v3/security/oob_buffer_view.gltf @@ -0,0 +1,9 @@ +{ + "asset": {"version": "2.0"}, + "buffers": [{"byteLength": 4}], + "bufferViews": [{"buffer": 0, "byteOffset": 0, "byteLength": 4}], + "accessors": [ + {"bufferView": 1000000, "byteOffset": 0, "componentType": 5121, "count": 1, "type": "SCALAR"} + ], + "scenes": [{"nodes": []}] +} diff --git a/tests/v3/security/oob_extension_indices.gltf b/tests/v3/security/oob_extension_indices.gltf new file mode 100644 index 0000000..2737ff1 --- /dev/null +++ b/tests/v3/security/oob_extension_indices.gltf @@ -0,0 +1,19 @@ +{ + "asset": {"version": "2.0"}, + "nodes": [ + {"extensions": {"KHR_audio": {"emitter": 2147483647}, + "MSFT_lod": {"ids": [-1, 9999]}}} + ], + "materials": [ + {"extensions": {"MSFT_lod": {"ids": [-5]}}} + ], + "scenes": [ + {"extensions": {"KHR_audio": {"emitters": [12345]}}} + ], + "extensions": { + "KHR_audio": { + "sources": [{"bufferView": -1}], + "emitters": [{"source": 99999}] + } + } +} diff --git a/tests/v3/security/traversal_relative.gltf b/tests/v3/security/traversal_relative.gltf new file mode 100644 index 0000000..92d4d44 --- /dev/null +++ b/tests/v3/security/traversal_relative.gltf @@ -0,0 +1,14 @@ +{ + "asset": {"version": "2.0"}, + "buffers": [ + {"uri": "../../../../../../../../tmp/tg3-poc-secret.txt", "byteLength": 32} + ], + "bufferViews": [ + {"buffer": 0, "byteOffset": 0, "byteLength": 32} + ], + "accessors": [ + {"bufferView": 0, "byteOffset": 0, "componentType": 5121, "count": 32, "type": "SCALAR"} + ], + "scenes": [{"nodes": []}], + "scene": 0 +} diff --git a/tiny_gltf_v3.c b/tiny_gltf_v3.c index 87a8a0b..400ca43 100644 --- a/tiny_gltf_v3.c +++ b/tiny_gltf_v3.c @@ -230,6 +230,7 @@ TINYGLTF3_API void tg3_parse_options_init(tg3_parse_options *options) { options->memory.memory_budget = TINYGLTF3_MAX_MEMORY_BYTES; options->memory.arena_block_size = TG3__ARENA_DEFAULT_BLOCK_SIZE; options->max_external_file_size = 0; + options->validate_indices = 1; } TINYGLTF3_API void tg3_write_options_init(tg3_write_options *options) { @@ -1008,6 +1009,32 @@ static int tg3__parse_accessor(tg3__parse_ctx *ctx, const tg3json_value *o, tg3_ return 1; } +/* Reject URIs that could escape base_dir or smuggle a different path through + * path-resolution layers. Returns 1 if the URI is safe to concatenate with + * base_dir, 0 if it must be rejected. */ +static int tg3__uri_is_safe(const char *uri, uint32_t uri_len) { + uint32_t i; + if (!uri || uri_len == 0) return 0; + /* No NUL bytes — fopen would truncate; protects against smuggling. */ + if (memchr(uri, '\0', uri_len) != NULL) return 0; + /* Reject absolute paths (POSIX and Windows). */ + if (uri[0] == '/' || uri[0] == '\\') return 0; + /* Reject Windows drive prefixes like "C:". */ + if (uri_len >= 2 && uri[1] == ':' && + ((uri[0] >= 'A' && uri[0] <= 'Z') || (uri[0] >= 'a' && uri[0] <= 'z'))) { + return 0; + } + /* Reject any ".." segment, separator-aware on both / and \. */ + for (i = 0; i + 1 < uri_len; ++i) { + if (uri[i] == '.' && uri[i + 1] == '.') { + int at_start = (i == 0) || uri[i - 1] == '/' || uri[i - 1] == '\\'; + int at_end = (i + 2 == uri_len) || uri[i + 2] == '/' || uri[i + 2] == '\\'; + if (at_start && at_end) return 0; + } + } + return 1; +} + static int tg3__load_external_file(tg3__parse_ctx *ctx, uint8_t **out_data, uint64_t *out_size, const char *uri, uint32_t uri_len) { char path_buf[4096]; @@ -1018,8 +1045,17 @@ static int tg3__load_external_file(tg3__parse_ctx *ctx, uint8_t **out_data, uint "No filesystem callbacks available", NULL, -1); return 0; } + if (!tg3__uri_is_safe(uri, uri_len)) { + tg3__error_push(ctx->errors, TG3_SEVERITY_ERROR, TG3_ERR_INVALID_VALUE, + "External URI rejected (absolute path, '..' segment, or NUL byte)", + NULL, -1); + return 0; + } + /* Saturating bounds check: path_buf is fixed-size; subtract instead of add + * so 32-bit size_t cannot wrap. */ + if (uri_len >= sizeof(path_buf)) return 0; if (ctx->base_dir_len > 0) { - if (ctx->base_dir_len + 1u + uri_len >= sizeof(path_buf)) return 0; + if (ctx->base_dir_len >= sizeof(path_buf) - uri_len - 1u) return 0; memcpy(path_buf, ctx->base_dir, ctx->base_dir_len); path_len = ctx->base_dir_len; if (path_len > 0 && path_buf[path_len - 1u] != '/' && path_buf[path_len - 1u] != '\\') { @@ -1107,6 +1143,17 @@ static int tg3__parse_buffer_view(tg3__parse_ctx *ctx, const tg3json_value *o, t tg3__parse_uint64(ctx, o, "byteLength", &val, 1, "/bufferView"); bv->byte_length = val; tg3__parse_int(ctx, o, "byteStride", &stride, 0, "/bufferView"); + /* glTF spec: byteStride is 0 (tightly packed) or in [4, 252]. Reject + * negatives so the (uint32_t) cast cannot wrap to 2^32-1 and propagate + * into downstream size math; reject 1..3 so consumers that pre-allocate + * `count * stride` cannot underallocate against a spec-non-conforming + * stride. */ + if (stride != 0 && (stride < 4 || stride > 252)) { + tg3__error_pushf(ctx->errors, ctx->arena, TG3_SEVERITY_ERROR, + TG3_ERR_INVALID_VALUE, "/bufferView", + "byteStride %d must be 0 or in [4, 252]", stride); + return 0; + } bv->byte_stride = (uint32_t)stride; tg3__parse_int(ctx, o, "target", &bv->target, 0, "/bufferView"); tg3__parse_extras_and_extensions(ctx, o, &bv->ext); @@ -1256,7 +1303,9 @@ static int tg3__parse_primitive(tg3__parse_ctx *ctx, const tg3json_value *o, tg3 } } target_arrays[ti] = tattrs; - target_counts[ti] = (uint32_t)acount; + /* On arena OOM tattrs may be NULL; keep count consistent so + * the index validator and downstream consumers don't deref. */ + target_counts[ti] = tattrs ? (uint32_t)acount : 0u; } prim->targets = target_arrays; prim->target_attribute_counts = target_counts; @@ -1546,6 +1595,203 @@ static int tg3__parse_audio_emitter(tg3__parse_ctx *ctx, const tg3json_value *o, } \ } while (0) +/* Post-parse index validation. Walks every int32_t index field populated from + * JSON and rejects out-of-range values so naive consumers cannot dereference + * attacker-controlled indices into model arrays. Caps reported errors so a + * pathological input does not flood the error stack. Returns 1 on success. */ +#define TG3__IDX_ERR_CAP 64 +static int tg3__validate_indices(tg3__parse_ctx *ctx, const tg3_model *m) { + uint32_t i, j; + int errs = 0; + /* Helper macros — push with format and bump error counter. */ + #define TG3__IDX_BAD(path_str, fmt, ...) do { \ + if (errs < TG3__IDX_ERR_CAP) { \ + tg3__error_pushf(ctx->errors, ctx->arena, TG3_SEVERITY_ERROR, \ + TG3_ERR_INVALID_INDEX, (path_str), fmt, ##__VA_ARGS__); \ + } \ + ++errs; \ + } while (0) + #define TG3__CHECK_REQ(idx, max, path_str, fmt, ...) do { \ + if ((idx) < 0 || (uint32_t)(idx) >= (max)) { \ + TG3__IDX_BAD(path_str, fmt, ##__VA_ARGS__); \ + } \ + } while (0) + #define TG3__CHECK_OPT(idx, max, path_str, fmt, ...) do { \ + if ((idx) != -1 && ((idx) < 0 || (uint32_t)(idx) >= (max))) { \ + TG3__IDX_BAD(path_str, fmt, ##__VA_ARGS__); \ + } \ + } while (0) + + if (m->default_scene != -1) { + TG3__CHECK_OPT(m->default_scene, m->scenes_count, "/scene", + "default scene %d out of range [0,%u)", + m->default_scene, m->scenes_count); + } + for (i = 0; i < m->buffer_views_count && errs < TG3__IDX_ERR_CAP; ++i) { + TG3__CHECK_REQ(m->buffer_views[i].buffer, m->buffers_count, + "/bufferViews", "bufferViews[%u].buffer %d out of range [0,%u)", + i, m->buffer_views[i].buffer, m->buffers_count); + } + for (i = 0; i < m->accessors_count && errs < TG3__IDX_ERR_CAP; ++i) { + const tg3_accessor *a = &m->accessors[i]; + TG3__CHECK_OPT(a->buffer_view, m->buffer_views_count, "/accessors", + "accessors[%u].bufferView %d out of range [0,%u)", + i, a->buffer_view, m->buffer_views_count); + if (a->sparse.is_sparse) { + TG3__CHECK_REQ(a->sparse.indices.buffer_view, m->buffer_views_count, + "/accessors", "accessors[%u].sparse.indices.bufferView %d out of range [0,%u)", + i, a->sparse.indices.buffer_view, m->buffer_views_count); + TG3__CHECK_REQ(a->sparse.values.buffer_view, m->buffer_views_count, + "/accessors", "accessors[%u].sparse.values.bufferView %d out of range [0,%u)", + i, a->sparse.values.buffer_view, m->buffer_views_count); + } + } + for (i = 0; i < m->meshes_count && errs < TG3__IDX_ERR_CAP; ++i) { + const tg3_mesh *me = &m->meshes[i]; + for (j = 0; j < me->primitives_count && errs < TG3__IDX_ERR_CAP; ++j) { + const tg3_primitive *p = &me->primitives[j]; + uint32_t ai; + TG3__CHECK_OPT(p->indices, m->accessors_count, "/meshes", + "meshes[%u].primitives[%u].indices %d out of range [0,%u)", + i, j, p->indices, m->accessors_count); + TG3__CHECK_OPT(p->material, m->materials_count, "/meshes", + "meshes[%u].primitives[%u].material %d out of range [0,%u)", + i, j, p->material, m->materials_count); + for (ai = 0; ai < p->attributes_count && errs < TG3__IDX_ERR_CAP; ++ai) { + TG3__CHECK_REQ(p->attributes[ai].value, m->accessors_count, "/meshes", + "meshes[%u].primitives[%u].attributes[%u] %d out of range [0,%u)", + i, j, ai, p->attributes[ai].value, m->accessors_count); + } + { + uint32_t ti; + for (ti = 0; ti < p->targets_count && errs < TG3__IDX_ERR_CAP; ++ti) { + uint32_t tk; + uint32_t tcount = p->target_attribute_counts ? p->target_attribute_counts[ti] : 0; + const tg3_str_int_pair *tarr = p->targets ? p->targets[ti] : NULL; + if (!tarr) continue; + for (tk = 0; tk < tcount && errs < TG3__IDX_ERR_CAP; ++tk) { + TG3__CHECK_REQ(tarr[tk].value, m->accessors_count, "/meshes", + "meshes[%u].primitives[%u].targets[%u][%u] %d out of range [0,%u)", + i, j, ti, tk, tarr[tk].value, m->accessors_count); + } + } + } + } + } + for (i = 0; i < m->nodes_count && errs < TG3__IDX_ERR_CAP; ++i) { + const tg3_node *n = &m->nodes[i]; + uint32_t k; + TG3__CHECK_OPT(n->mesh, m->meshes_count, "/nodes", "nodes[%u].mesh %d out of range [0,%u)", i, n->mesh, m->meshes_count); + TG3__CHECK_OPT(n->skin, m->skins_count, "/nodes", "nodes[%u].skin %d out of range [0,%u)", i, n->skin, m->skins_count); + TG3__CHECK_OPT(n->camera, m->cameras_count, "/nodes", "nodes[%u].camera %d out of range [0,%u)", i, n->camera, m->cameras_count); + TG3__CHECK_OPT(n->light, m->lights_count, "/nodes", "nodes[%u].light %d out of range [0,%u)", i, n->light, m->lights_count); + for (k = 0; k < n->children_count && errs < TG3__IDX_ERR_CAP; ++k) { + TG3__CHECK_REQ(n->children[k], m->nodes_count, "/nodes", + "nodes[%u].children[%u] %d out of range [0,%u)", + i, k, n->children[k], m->nodes_count); + } + } + for (i = 0; i < m->textures_count && errs < TG3__IDX_ERR_CAP; ++i) { + TG3__CHECK_OPT(m->textures[i].source, m->images_count, "/textures", + "textures[%u].source %d out of range [0,%u)", i, m->textures[i].source, m->images_count); + TG3__CHECK_OPT(m->textures[i].sampler, m->samplers_count, "/textures", + "textures[%u].sampler %d out of range [0,%u)", i, m->textures[i].sampler, m->samplers_count); + } + for (i = 0; i < m->images_count && errs < TG3__IDX_ERR_CAP; ++i) { + TG3__CHECK_OPT(m->images[i].buffer_view, m->buffer_views_count, "/images", + "images[%u].bufferView %d out of range [0,%u)", + i, m->images[i].buffer_view, m->buffer_views_count); + } + for (i = 0; i < m->skins_count && errs < TG3__IDX_ERR_CAP; ++i) { + const tg3_skin *s = &m->skins[i]; + uint32_t k; + TG3__CHECK_OPT(s->inverse_bind_matrices, m->accessors_count, "/skins", + "skins[%u].inverseBindMatrices %d out of range [0,%u)", + i, s->inverse_bind_matrices, m->accessors_count); + TG3__CHECK_OPT(s->skeleton, m->nodes_count, "/skins", + "skins[%u].skeleton %d out of range [0,%u)", + i, s->skeleton, m->nodes_count); + for (k = 0; k < s->joints_count && errs < TG3__IDX_ERR_CAP; ++k) { + TG3__CHECK_REQ(s->joints[k], m->nodes_count, "/skins", + "skins[%u].joints[%u] %d out of range [0,%u)", + i, k, s->joints[k], m->nodes_count); + } + } + for (i = 0; i < m->animations_count && errs < TG3__IDX_ERR_CAP; ++i) { + const tg3_animation *an = &m->animations[i]; + uint32_t k; + for (k = 0; k < an->channels_count && errs < TG3__IDX_ERR_CAP; ++k) { + TG3__CHECK_REQ(an->channels[k].sampler, an->samplers_count, "/animations", + "animations[%u].channels[%u].sampler %d out of range [0,%u)", + i, k, an->channels[k].sampler, an->samplers_count); + TG3__CHECK_OPT(an->channels[k].target.node, m->nodes_count, "/animations", + "animations[%u].channels[%u].target.node %d out of range [0,%u)", + i, k, an->channels[k].target.node, m->nodes_count); + } + for (k = 0; k < an->samplers_count && errs < TG3__IDX_ERR_CAP; ++k) { + TG3__CHECK_REQ(an->samplers[k].input, m->accessors_count, "/animations", + "animations[%u].samplers[%u].input %d out of range [0,%u)", + i, k, an->samplers[k].input, m->accessors_count); + TG3__CHECK_REQ(an->samplers[k].output, m->accessors_count, "/animations", + "animations[%u].samplers[%u].output %d out of range [0,%u)", + i, k, an->samplers[k].output, m->accessors_count); + } + } + for (i = 0; i < m->scenes_count && errs < TG3__IDX_ERR_CAP; ++i) { + uint32_t k; + const tg3_scene *s = &m->scenes[i]; + for (k = 0; k < s->nodes_count && errs < TG3__IDX_ERR_CAP; ++k) { + TG3__CHECK_REQ(s->nodes[k], m->nodes_count, "/scenes", + "scenes[%u].nodes[%u] %d out of range [0,%u)", + i, k, s->nodes[k], m->nodes_count); + } + for (k = 0; k < s->audio_emitters_count && errs < TG3__IDX_ERR_CAP; ++k) { + TG3__CHECK_REQ(s->audio_emitters[k], m->audio_emitters_count, "/scenes", + "scenes[%u].audio_emitters[%u] %d out of range [0,%u)", + i, k, s->audio_emitters[k], m->audio_emitters_count); + } + } + /* Extension index fields (KHR_audio, MSFT_lod). */ + for (i = 0; i < m->nodes_count && errs < TG3__IDX_ERR_CAP; ++i) { + const tg3_node *n = &m->nodes[i]; + uint32_t k; + TG3__CHECK_OPT(n->emitter, m->audio_emitters_count, "/nodes", + "nodes[%u].emitter %d out of range [0,%u)", + i, n->emitter, m->audio_emitters_count); + for (k = 0; k < n->lods_count && errs < TG3__IDX_ERR_CAP; ++k) { + TG3__CHECK_REQ(n->lods[k], m->nodes_count, "/nodes", + "nodes[%u].lods[%u] %d out of range [0,%u)", + i, k, n->lods[k], m->nodes_count); + } + } + for (i = 0; i < m->materials_count && errs < TG3__IDX_ERR_CAP; ++i) { + const tg3_material *mat = &m->materials[i]; + uint32_t k; + for (k = 0; k < mat->lods_count && errs < TG3__IDX_ERR_CAP; ++k) { + TG3__CHECK_REQ(mat->lods[k], m->materials_count, "/materials", + "materials[%u].lods[%u] %d out of range [0,%u)", + i, k, mat->lods[k], m->materials_count); + } + } + for (i = 0; i < m->audio_sources_count && errs < TG3__IDX_ERR_CAP; ++i) { + TG3__CHECK_OPT(m->audio_sources[i].buffer_view, m->buffer_views_count, + "/extensions/KHR_audio/sources", + "audio_sources[%u].bufferView %d out of range [0,%u)", + i, m->audio_sources[i].buffer_view, m->buffer_views_count); + } + for (i = 0; i < m->audio_emitters_count && errs < TG3__IDX_ERR_CAP; ++i) { + TG3__CHECK_OPT(m->audio_emitters[i].source, m->audio_sources_count, + "/extensions/KHR_audio/emitters", + "audio_emitters[%u].source %d out of range [0,%u)", + i, m->audio_emitters[i].source, m->audio_sources_count); + } + + #undef TG3__CHECK_OPT + #undef TG3__CHECK_REQ + #undef TG3__IDX_BAD + return errs == 0; +} + static tg3_error_code tg3__parse_from_json(tg3__parse_ctx *ctx, const tg3json_value *json_doc, tg3_model *model) { const tg3json_value *asset_it = tg3json_object_get(json_doc, "asset"); const tg3json_value *ext_it; @@ -1606,6 +1852,11 @@ static tg3_error_code tg3__parse_from_json(tg3__parse_ctx *ctx, const tg3json_va } } tg3__parse_extras_and_extensions(ctx, json_doc, &model->ext); + if (ctx->opts.validate_indices) { + if (!tg3__validate_indices(ctx, model)) { + return TG3_ERR_INVALID_INDEX; + } + } return (ctx->errors && ctx->errors->has_error) ? TG3_ERR_JSON_PARSE : TG3_OK; } @@ -1692,7 +1943,8 @@ TINYGLTF3_API tg3_error_code tg3_parse(tg3_model *model, tg3_error_stack *errors tg3__error_push(errors, TG3_SEVERITY_ERROR, TG3_ERR_JSON_PARSE, "Failed to parse JSON", NULL, error_pos ? (int64_t)(error_pos - (const char *)json_data) : -1); if (parsed_ok) tg3json_value_free(&json_doc); - if (model->arena_) { tg3__arena_destroy(model->arena_); model->arena_ = NULL; } + /* Keep arena alive so error messages (arena-allocated) stay valid for + * the caller; tg3_model_free is the sole arena owner on the error path. */ return TG3_ERR_JSON_PARSE; } memset(&ctx, 0, sizeof(ctx)); @@ -1706,10 +1958,9 @@ TINYGLTF3_API tg3_error_code tg3_parse(tg3_model *model, tg3_error_stack *errors #endif ret = tg3__parse_from_json(&ctx, &json_doc, model); tg3json_value_free(&json_doc); - if (ret != TG3_OK && model->arena_) { - tg3__arena_destroy(model->arena_); - model->arena_ = NULL; - } + /* Arena stays attached to model->arena_ on both success and failure; + * tg3_model_free reclaims it. Destroying here would dangle arena-allocated + * error messages on the user-facing tg3_error_stack. */ return ret; } @@ -1728,10 +1979,13 @@ TINYGLTF3_API tg3_error_code tg3_parse_glb(tg3_model *model, tg3_error_stack *er const char *error_pos = NULL; tg3_error_code err; int parsed_ok; + /* Initialize model before any failure-return so callers can safely call + * tg3_model_free() on the error path; the GLB header parse must not run + * against a model whose arena_ field is uninitialized garbage. */ + tg3__model_init(model); err = tg3__parse_glb_header(glb_data, glb_size, &json_chunk, &json_chunk_size, &bin_chunk, &bin_chunk_size, errors); if (err != TG3_OK) return err; if (!options) { tg3_parse_options_init(&default_opts); options = &default_opts; } - tg3__model_init(model); arena = tg3__arena_create(&options->memory); if (!arena) { tg3__error_push(errors, TG3_SEVERITY_ERROR, TG3_ERR_OUT_OF_MEMORY, "Failed to create arena", NULL, -1); @@ -1744,8 +1998,7 @@ TINYGLTF3_API tg3_error_code tg3_parse_glb(tg3_model *model, tg3_error_stack *er tg3__error_push(errors, TG3_SEVERITY_ERROR, TG3_ERR_JSON_PARSE, "Failed to parse GLB JSON chunk", NULL, error_pos ? (int64_t)(error_pos - (const char *)json_chunk) : -1); if (parsed_ok) tg3json_value_free(&json_doc); - tg3__arena_destroy(model->arena_); - model->arena_ = NULL; + /* Keep arena alive so error messages stay valid; model_free owns it. */ return TG3_ERR_JSON_PARSE; } memset(&ctx, 0, sizeof(ctx)); @@ -1762,10 +2015,7 @@ TINYGLTF3_API tg3_error_code tg3_parse_glb(tg3_model *model, tg3_error_stack *er #endif err = tg3__parse_from_json(&ctx, &json_doc, model); tg3json_value_free(&json_doc); - if (err != TG3_OK && model->arena_) { - tg3__arena_destroy(model->arena_); - model->arena_ = NULL; - } + /* Arena stays attached to model->arena_ on both paths; model_free owns it. */ return err; } diff --git a/tiny_gltf_v3.h b/tiny_gltf_v3.h index a8234da..7bbdadb 100644 --- a/tiny_gltf_v3.h +++ b/tiny_gltf_v3.h @@ -37,6 +37,37 @@ * - Streaming parse/write via callbacks * - No RTTI, no exceptions required * - C++20 coroutine facade (optional) + * + * Security considerations (read before processing untrusted glTF): + * + * 1. External URI loading. When TINYGLTF3_ENABLE_FS is defined and no custom + * tg3_fs_callbacks are supplied, the parser opens external buffer/image + * URIs through the libc default fopen(). The parser rejects URIs that + * contain '..' segments, leading '/' or '\\', Windows drive prefixes + * (e.g. "C:"), or NUL bytes — but it does NOT chroot or canonicalize the + * result. Production callers SHOULD provide a tg3_fs_callbacks with a + * read_file callback that confines reads to a known directory (e.g. via + * openat(AT_FDCWD, path, O_NOFOLLOW) plus a realpath() prefix check) when + * the input glTF is attacker-controlled. + * + * 2. Index validation. Many glTF fields are integer indices into model + * arrays (accessor.bufferView, primitive.material, scene.nodes[], etc.). + * With opts.validate_indices = 1 (the default) the parser rejects every + * out-of-range index after the structural parse and returns + * TG3_ERR_INVALID_INDEX. Set opts.validate_indices = 0 only when you + * need to round-trip raw or extension data and have your own validator. + * + * 3. Image decoding. The parser does not decode image bytes by default. + * Set opts.images_as_is = 1 (already the safe default for untrusted + * input) to skip any decoder and store raw bytes only. + * + * 4. Memory budget. The arena is capped at TINYGLTF3_MAX_MEMORY_BYTES + * (1 GB by default; configurable per-parse via tg3_memory_config). + * The parser returns TG3_ERR_OUT_OF_MEMORY rather than overcommitting. + * + * 5. Error message lifetime. Error strings on tg3_error_stack are + * arena-allocated and remain valid until tg3_model_free() is called. + * Read or copy them BEFORE freeing the model. */ #ifndef TINY_GLTF_V3_H_ @@ -914,6 +945,10 @@ typedef struct tg3_parse_options { * (breaks strict double-precision conformance * but sufficient for glTF data which is * typically single-precision anyway) */ + int32_t validate_indices; /* 1 = reject out-of-range index fields + * after parse so naive consumers cannot + * dereference attacker-controlled indices. + * Default: 1. Set to 0 to skip (raw mode). */ uint64_t max_external_file_size; /* 0 = no limit */ } tg3_parse_options;