From 18482d7ceea7b1407ee4149c79468a2c5aeed9b9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 13:19:42 +0000 Subject: [PATCH 01/26] Initial plan From 3652d8de86924ee3390511713637e2b8a6bea486 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 13:25:13 +0000 Subject: [PATCH 02/26] fix: pass percent-decoded URI to resolveUri callback and update JSDoc Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com> --- src/core/GltfLoader.ts | 15 +++++++++++++-- tests/gltf.test.ts | 28 ++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/core/GltfLoader.ts b/src/core/GltfLoader.ts index 2191756..1b5f339 100644 --- a/src/core/GltfLoader.ts +++ b/src/core/GltfLoader.ts @@ -126,8 +126,13 @@ function safeParseGltfJson(text: string): GltfAsset { export interface GltfLoaderOptions { /** * Callback invoked to resolve external buffer URIs referenced by the glTF asset. - * Receives the raw URI string and must return the corresponding binary data. + * Receives the percent-decoded URI string and must return the corresponding binary data. * Required when loading plain `.gltf` files that reference external `.bin` files. + * + * **Security warning**: the URI received by this callback has already been validated + * and percent-decoded by the loader. Do not perform additional URI resolution or + * decoding inside this callback — doing so may re-introduce path-traversal or + * SSRF vulnerabilities that the loader's validation was designed to prevent. */ resolveUri?: (uri: string) => Promise; /** @@ -434,7 +439,13 @@ async function resolveBuffers( ); } validateExternalUri(buf.uri, i, options?.strict); - const externalBuffer = await resolveUri(buf.uri); + // Pass the percent-decoded URI to the callback so consumers never receive + // a raw percent-encoded string that could be re-decoded to a dangerous path. + // The fallback to the original URI on decode failure is safe because + // validateExternalUri has already checked both raw and decoded forms. + let safeUri: string; + try { safeUri = decodeURIComponent(buf.uri); } catch { safeUri = buf.uri; } + const externalBuffer = await resolveUri(safeUri); assertByteLength(externalBuffer, buf.byteLength, i); resolved.push(externalBuffer); } diff --git a/tests/gltf.test.ts b/tests/gltf.test.ts index d56f9a6..2f3f478 100644 --- a/tests/gltf.test.ts +++ b/tests/gltf.test.ts @@ -1199,6 +1199,26 @@ describe('loadGltf', () => { expect(resolveUri).toHaveBeenCalledWith('my file.bin'); }); + it('passes percent-decoded URI to resolveUri callback for safe encoded URIs', async () => { + const { json, bin } = triangleAsset(); + // %6d%6f%64%65%6c%73%2ftriangle%2ebin decodes to "models/triangle.bin" + json.buffers = [{ uri: '%6d%6f%64%65%6c%73%2ftriangle%2ebin', byteLength: bin.byteLength }]; + const buffer = jsonToBuffer(json); + const resolveUri = vi.fn().mockResolvedValue(bin); + await expect(loadGltf(buffer, { resolveUri })).resolves.toMatchObject({ meshes: expect.any(Array) }); + // The callback must receive the decoded form, not the raw percent-encoded string + expect(resolveUri).toHaveBeenCalledWith('models/triangle.bin'); + }); + + it('rejects encoded bypass "%2e%2e/secret" before it reaches the resolveUri callback', async () => { + const { json, bin } = triangleAsset(); + json.buffers = [{ uri: '%2e%2e/secret', byteLength: bin.byteLength }]; + const buffer = jsonToBuffer(json); + const resolveUri = vi.fn().mockResolvedValue(bin); + await expect(loadGltf(buffer, { resolveUri })).rejects.toThrow(/Only relative paths without traversal/); + expect(resolveUri).not.toHaveBeenCalled(); + }); + it('handles meshes with normals and UVs', async () => { // 3 vertices × (3 pos + 3 normal + 2 uv) = 3×8 = 24 floats = 96 bytes // 3 indices = 6 bytes @@ -2114,4 +2134,12 @@ describe('GltfLoaderOptions JSDoc', () => { it('strict JSDoc documents the false default value', () => { expect(gltfLoaderSource).toContain('(default) in production'); }); + + it('resolveUri JSDoc warns consumers not to perform additional URI resolution', () => { + expect(gltfLoaderSource).toContain('Do not perform additional URI resolution'); + }); + + it('resolveUri JSDoc documents that the URI is percent-decoded before being passed', () => { + expect(gltfLoaderSource).toContain('percent-decoded'); + }); }); From 75c623d77c4ca4895010f2246af0397a1edb3333 Mon Sep 17 00:00:00 2001 From: Hugo Doueil <46538211+ormidales@users.noreply.github.com> Date: Fri, 13 Mar 2026 14:34:54 +0100 Subject: [PATCH 03/26] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/core/GltfLoader.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/core/GltfLoader.ts b/src/core/GltfLoader.ts index 1b5f339..3dd500c 100644 --- a/src/core/GltfLoader.ts +++ b/src/core/GltfLoader.ts @@ -126,13 +126,17 @@ function safeParseGltfJson(text: string): GltfAsset { export interface GltfLoaderOptions { /** * Callback invoked to resolve external buffer URIs referenced by the glTF asset. - * Receives the percent-decoded URI string and must return the corresponding binary data. + * Receives a URI string that the loader has attempted to percent-decode and must + * return the corresponding binary data. If the URI contains invalid percent-encoding, + * decoding may fail and the original (possibly still percent-encoded) URI string + * will be passed to this callback. * Required when loading plain `.gltf` files that reference external `.bin` files. * * **Security warning**: the URI received by this callback has already been validated - * and percent-decoded by the loader. Do not perform additional URI resolution or - * decoding inside this callback — doing so may re-introduce path-traversal or - * SSRF vulnerabilities that the loader's validation was designed to prevent. + * and normalization / percent-decoding have been applied on a best-effort basis by + * the loader. Do not perform additional URI resolution or decoding inside this + * callback — doing so may re-introduce path-traversal or SSRF vulnerabilities that + * the loader's validation was designed to prevent. */ resolveUri?: (uri: string) => Promise; /** From 2c06092e230cc42a92b6b6e930407f127f17b1d8 Mon Sep 17 00:00:00 2001 From: Hugo Doueil <46538211+ormidales@users.noreply.github.com> Date: Fri, 13 Mar 2026 14:35:43 +0100 Subject: [PATCH 04/26] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/core/GltfLoader.ts | 49 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/src/core/GltfLoader.ts b/src/core/GltfLoader.ts index 3dd500c..2fe51ca 100644 --- a/src/core/GltfLoader.ts +++ b/src/core/GltfLoader.ts @@ -407,6 +407,40 @@ async function resolveBuffers( } }; + /** + * Fully percent-decode a URI in a bounded loop. + * + * This prevents double-encoded traversal/scheme payloads from slipping + * through validation when consumers perform an additional decode. + * + * If further decoding would fail (malformed escape sequences), the last + * successfully decoded value is used. After the loop, if the string still + * contains percent-encoded bytes, the URI is rejected as suspicious. + */ + const fullyDecodeUri = (uri: string, maxIterations = 5): string => { + let current = uri; + for (let i = 0; i < maxIterations; i++) { + let decoded: string; + try { + decoded = decodeURIComponent(current); + } catch { + // Stop decoding on failure and use the last valid value. + break; + } + if (decoded === current) { + break; + } + current = decoded; + } + // If there are still percent-encoded octets present, treat as suspicious. + if (/%[0-9a-fA-F]{2}/.test(current)) { + throw new Error( + `Rejected suspicious percent-encoded URI after decoding: ${JSON.stringify(current)}`, + ); + } + return current; + }; + let binChunkConsumed = false; for (let i = 0; i < gltfBuffers.length; i++) { const buf = gltfBuffers[i]; @@ -442,14 +476,15 @@ async function resolveBuffers( `Buffer ${i} references external URI "${buf.uri}" but no resolveUri callback was provided.`, ); } + // Validate the original URI as authored in the glTF asset. validateExternalUri(buf.uri, i, options?.strict); - // Pass the percent-decoded URI to the callback so consumers never receive - // a raw percent-encoded string that could be re-decoded to a dangerous path. - // The fallback to the original URI on decode failure is safe because - // validateExternalUri has already checked both raw and decoded forms. - let safeUri: string; - try { safeUri = decodeURIComponent(buf.uri); } catch { safeUri = buf.uri; } - const externalBuffer = await resolveUri(safeUri); + // Fully percent-decode the URI in a bounded loop so that any traversal + // or scheme payload only expressed after multiple decodes is exposed. + const fullyDecodedUri = fullyDecodeUri(buf.uri); + // Validate the fully decoded form as well, since this is what will be + // passed to the resolveUri callback and potentially used by consumers. + validateExternalUri(fullyDecodedUri, i, options?.strict); + const externalBuffer = await resolveUri(fullyDecodedUri); assertByteLength(externalBuffer, buf.byteLength, i); resolved.push(externalBuffer); } From 4d94ef78be0cc2dd47467d6efc9f3984bd147d74 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 13:37:04 +0000 Subject: [PATCH 05/26] Initial plan From aecdceade6799098779a6bad8e1cc627bd585843 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 13:41:54 +0000 Subject: [PATCH 06/26] fix: use FNV-1a hash as shader cache key instead of raw GLSL source Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com> --- src/core/ShaderCache.ts | 16 ++++++++++++---- tests/shader.test.ts | 21 +++++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/core/ShaderCache.ts b/src/core/ShaderCache.ts index 66daa9e..a2d7e10 100644 --- a/src/core/ShaderCache.ts +++ b/src/core/ShaderCache.ts @@ -64,6 +64,14 @@ export class ShaderCache { return `fnv1a2-${ShaderCache.fnv1aSources(vertSrc, fragSrc, ShaderCache.FNV1A_OFFSET_BASIS_2)}`; } + /** + * Compute a compact, fixed-length cache key for a single GLSL source string + * using FNV-1a so the raw source is never stored as a Map key. + */ + private static hashShaderSource(source: string): string { + return `fnv1a-shader-${ShaderCache.fnv1aSources(source, '', ShaderCache.FNV1A_OFFSET_BASIS)}`; + } + /** key → compiled WebGLShader */ private readonly shaders: Map = new Map(); @@ -93,10 +101,10 @@ export class ShaderCache { * * @param type `gl.VERTEX_SHADER` or `gl.FRAGMENT_SHADER` * @param source GLSL source code - * @param key Optional cache key. Defaults to the source string itself. + * @param key Optional cache key. Defaults to an FNV-1a hash of the source string. */ getShader(type: number, source: string, key?: string): WebGLShader { - const cacheKey = key ?? source; + const cacheKey = key ?? ShaderCache.hashShaderSource(source); const existing = this.shaders.get(cacheKey); if (existing) return existing; @@ -171,8 +179,8 @@ export class ShaderCache { cacheKey: string, secondaryKey: string | undefined, ): WebGLProgram { - const vertexShaderKey = vertexSource; - const fragmentShaderKey = fragmentSource; + const vertexShaderKey = ShaderCache.hashShaderSource(vertexSource); + const fragmentShaderKey = ShaderCache.hashShaderSource(fragmentSource); const vsPreExisted = this.shaders.has(vertexShaderKey); const fsPreExisted = this.shaders.has(fragmentShaderKey); let vs: WebGLShader; diff --git a/tests/shader.test.ts b/tests/shader.test.ts index 14c8c63..46cd4db 100644 --- a/tests/shader.test.ts +++ b/tests/shader.test.ts @@ -214,6 +214,27 @@ describe('ShaderCache', () => { expect(gl.createShader).toHaveBeenCalledTimes(1); }); + it('uses a hash string, not the raw source, as the default shader cache key', () => { + let shaderId = 0; + (gl.createShader as ReturnType).mockImplementation( + () => ({ __shaderId: shaderId++ }) as unknown as WebGLShader, + ); + + // First call without an explicit key caches under a hash-derived key. + const s1 = cache.getShader(gl.VERTEX_SHADER, 'void main(){}'); + + // A second call with the same source returns the cached shader (hash hit). + const s2 = cache.getShader(gl.VERTEX_SHADER, 'void main(){}'); + expect(s1).toBe(s2); + expect(gl.createShader).toHaveBeenCalledTimes(1); + + // Passing the raw source string as an explicit key is a different slot: + // the hash key !== the source string, so a new shader is compiled. + const s3 = cache.getShader(gl.VERTEX_SHADER, 'void main(){}', 'void main(){}'); + expect(s3).not.toBe(s1); + expect(gl.createShader).toHaveBeenCalledTimes(2); + }); + it('supports a custom cache key for getProgram', () => { const p1 = cache.getProgram('v', 'f', 'prog-key'); const p2 = cache.getProgram('v', 'f', 'prog-key'); From f545e94ffe8ff4548b9411e14ac6a0bbc841d49d Mon Sep 17 00:00:00 2001 From: Hugo Doueil <46538211+ormidales@users.noreply.github.com> Date: Fri, 13 Mar 2026 14:54:21 +0100 Subject: [PATCH 07/26] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/core/ShaderCache.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/core/ShaderCache.ts b/src/core/ShaderCache.ts index a2d7e10..7558bd6 100644 --- a/src/core/ShaderCache.ts +++ b/src/core/ShaderCache.ts @@ -66,10 +66,14 @@ export class ShaderCache { /** * Compute a compact, fixed-length cache key for a single GLSL source string - * using FNV-1a so the raw source is never stored as a Map key. + * using a composite of two independent 32-bit FNV-1a hashes so the raw + * source is never stored as a Map key while achieving ~64-bit effective + * collision resistance. */ private static hashShaderSource(source: string): string { - return `fnv1a-shader-${ShaderCache.fnv1aSources(source, '', ShaderCache.FNV1A_OFFSET_BASIS)}`; + const primary = ShaderCache.fnv1aSources(source, '', ShaderCache.FNV1A_OFFSET_BASIS); + const secondary = ShaderCache.fnv1aSources(source, '', ShaderCache.FNV1A_OFFSET_BASIS_2); + return `fnv1a-shader-${primary}-${secondary}`; } /** key → compiled WebGLShader */ From 1d0517b6c1446ff91a0fbb0d55249ebf5944f4fa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 13:55:15 +0000 Subject: [PATCH 08/26] Initial plan From 12b9d92ec5aab0622b866996f658bed88e85ee41 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 14:02:08 +0000 Subject: [PATCH 09/26] fix: enforce maxJsonBufferBytes on decoded string before JSON.parse Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com> --- src/core/GltfLoader.ts | 12 +++++++++++- tests/gltf.test.ts | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/core/GltfLoader.ts b/src/core/GltfLoader.ts index 2fe51ca..112db5a 100644 --- a/src/core/GltfLoader.ts +++ b/src/core/GltfLoader.ts @@ -140,7 +140,11 @@ export interface GltfLoaderOptions { */ resolveUri?: (uri: string) => Promise; /** - * Maximum accepted byte size for a plain JSON glTF payload. + * Maximum accepted size for a plain JSON glTF payload, enforced on both the + * raw `ArrayBuffer` byte length **and** the decoded UTF-8 string character + * count before `JSON.parse` is called. Applying the limit to the decoded + * string prevents a maliciously crafted payload from exhausting the JS heap + * inside `JSON.parse` even if the byte-length guard were somehow bypassed. * Defaults to 64 MiB. Raise this value only when loading unusually large assets. */ maxJsonBufferBytes?: number; @@ -257,6 +261,12 @@ export function parseContainer( ); } const text = UTF8_DECODER.decode(buffer); + if (text.length > maxJsonBufferBytes) { + throw new Error( + `JSON glTF payload too large (${text.length} characters). ` + + `Maximum supported size is ${maxJsonBufferBytes} characters.`, + ); + } const json = safeParseGltfJson(text); return { json, binChunk: undefined }; } diff --git a/tests/gltf.test.ts b/tests/gltf.test.ts index 2f3f478..8edaa86 100644 --- a/tests/gltf.test.ts +++ b/tests/gltf.test.ts @@ -173,6 +173,25 @@ describe('parseContainer', () => { expect(() => parseContainer(oversized)).toThrow(/payload too large/); }); + it('rejects decoded JSON string exceeding maxJsonBufferBytes before JSON.parse is called', () => { + const json = JSON.stringify(minimalGltf()); + const buf = new TextEncoder().encode(json).buffer as ArrayBuffer; + const parseSpy = vi.spyOn(JSON, 'parse'); + try { + // maxJsonBufferBytes set to one less than the payload length — both the + // buffer-byte check and the decoded-string check should fire before + // JSON.parse is reached. + parseContainer(buf, { maxJsonBufferBytes: json.length - 1 }); + } catch { + // expected to throw + } + expect(parseSpy).not.toHaveBeenCalled(); + parseSpy.mockRestore(); + + // At the exact payload length the parse should succeed. + expect(() => parseContainer(buf, { maxJsonBufferBytes: json.length })).not.toThrow(); + }); + it('parses GLB container with JSON + BIN chunks', () => { const { json: srcJson, bin } = triangleAsset(); const glb = buildGlb(srcJson, bin); From 0e0f08bc662e5823d98107368e5eebbf7e36d4d6 Mon Sep 17 00:00:00 2001 From: Hugo Doueil <46538211+ormidales@users.noreply.github.com> Date: Fri, 13 Mar 2026 15:12:31 +0100 Subject: [PATCH 10/26] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/core/GltfLoader.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/core/GltfLoader.ts b/src/core/GltfLoader.ts index 112db5a..9914d11 100644 --- a/src/core/GltfLoader.ts +++ b/src/core/GltfLoader.ts @@ -140,11 +140,12 @@ export interface GltfLoaderOptions { */ resolveUri?: (uri: string) => Promise; /** - * Maximum accepted size for a plain JSON glTF payload, enforced on both the - * raw `ArrayBuffer` byte length **and** the decoded UTF-8 string character - * count before `JSON.parse` is called. Applying the limit to the decoded - * string prevents a maliciously crafted payload from exhausting the JS heap - * inside `JSON.parse` even if the byte-length guard were somehow bypassed. + * Maximum accepted size (in bytes) for a plain JSON glTF payload. This limit is + * enforced both on the raw `ArrayBuffer` byte length **and** on the length of the + * decoded JavaScript string (measured in UTF-16 code units via `text.length`) + * before `JSON.parse` is called. Bounding the decoded string length in this way + * helps prevent a maliciously crafted payload from passing an unreasonably large + * string to `JSON.parse`, even if the byte-length guard were somehow bypassed. * Defaults to 64 MiB. Raise this value only when loading unusually large assets. */ maxJsonBufferBytes?: number; From 3218c0a1fa4011c5cb911267eadf66425655bc65 Mon Sep 17 00:00:00 2001 From: Hugo Doueil <46538211+ormidales@users.noreply.github.com> Date: Fri, 13 Mar 2026 15:12:40 +0100 Subject: [PATCH 11/26] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- tests/gltf.test.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/tests/gltf.test.ts b/tests/gltf.test.ts index 8edaa86..1252145 100644 --- a/tests/gltf.test.ts +++ b/tests/gltf.test.ts @@ -178,15 +178,18 @@ describe('parseContainer', () => { const buf = new TextEncoder().encode(json).buffer as ArrayBuffer; const parseSpy = vi.spyOn(JSON, 'parse'); try { - // maxJsonBufferBytes set to one less than the payload length — both the - // buffer-byte check and the decoded-string check should fire before - // JSON.parse is reached. - parseContainer(buf, { maxJsonBufferBytes: json.length - 1 }); - } catch { - // expected to throw + try { + // maxJsonBufferBytes set to one less than the payload length — both the + // buffer-byte check and the decoded-string check should fire before + // JSON.parse is reached. + parseContainer(buf, { maxJsonBufferBytes: json.length - 1 }); + } catch { + // expected to throw + } + expect(parseSpy).not.toHaveBeenCalled(); + } finally { + parseSpy.mockRestore(); } - expect(parseSpy).not.toHaveBeenCalled(); - parseSpy.mockRestore(); // At the exact payload length the parse should succeed. expect(() => parseContainer(buf, { maxJsonBufferBytes: json.length })).not.toThrow(); From 2401701e023e6564fd1bf1d689fdb89a8984b202 Mon Sep 17 00:00:00 2001 From: Hugo Doueil <46538211+ormidales@users.noreply.github.com> Date: Fri, 13 Mar 2026 15:13:09 +0100 Subject: [PATCH 12/26] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/core/GltfLoader.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/core/GltfLoader.ts b/src/core/GltfLoader.ts index 9914d11..be75627 100644 --- a/src/core/GltfLoader.ts +++ b/src/core/GltfLoader.ts @@ -262,10 +262,11 @@ export function parseContainer( ); } const text = UTF8_DECODER.decode(buffer); - if (text.length > maxJsonBufferBytes) { + // Approximate heap usage of the decoded UTF-16 string (2 bytes per code unit) + if (text.length * 2 > maxJsonBufferBytes) { throw new Error( - `JSON glTF payload too large (${text.length} characters). ` + - `Maximum supported size is ${maxJsonBufferBytes} characters.`, + `JSON glTF string too large (~${text.length * 2} UTF-16 bytes). ` + + `Maximum supported decoded size is ${maxJsonBufferBytes} bytes.`, ); } const json = safeParseGltfJson(text); From 21c616394445c9529311a0c0dbc87b924cb53640 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 14:19:34 +0000 Subject: [PATCH 13/26] fix: correct test thresholds and JSDoc units for text.length*2 guard Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com> --- src/core/GltfLoader.ts | 16 +++++++++++----- tests/gltf.test.ts | 18 +++++++++--------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/core/GltfLoader.ts b/src/core/GltfLoader.ts index be75627..5ff41c5 100644 --- a/src/core/GltfLoader.ts +++ b/src/core/GltfLoader.ts @@ -141,11 +141,17 @@ export interface GltfLoaderOptions { resolveUri?: (uri: string) => Promise; /** * Maximum accepted size (in bytes) for a plain JSON glTF payload. This limit is - * enforced both on the raw `ArrayBuffer` byte length **and** on the length of the - * decoded JavaScript string (measured in UTF-16 code units via `text.length`) - * before `JSON.parse` is called. Bounding the decoded string length in this way - * helps prevent a maliciously crafted payload from passing an unreasonably large - * string to `JSON.parse`, even if the byte-length guard were somehow bypassed. + * enforced at two points: + * + * 1. On the raw `ArrayBuffer` byte length before decoding. + * 2. On the approximate in-memory UTF-16 footprint of the decoded string + * (`text.length * 2`) before `JSON.parse` is called. + * + * Because a JavaScript string stores each code unit as two bytes (UTF-16), a + * 27-byte ASCII JSON payload produces a string whose heap footprint is + * approximately 54 bytes. Callers should set this limit to at least twice the + * expected source byte size when dealing with predominantly ASCII content. + * * Defaults to 64 MiB. Raise this value only when loading unusually large assets. */ maxJsonBufferBytes?: number; diff --git a/tests/gltf.test.ts b/tests/gltf.test.ts index 1252145..54f9477 100644 --- a/tests/gltf.test.ts +++ b/tests/gltf.test.ts @@ -174,25 +174,25 @@ describe('parseContainer', () => { }); it('rejects decoded JSON string exceeding maxJsonBufferBytes before JSON.parse is called', () => { - const json = JSON.stringify(minimalGltf()); + const json = JSON.stringify(minimalGltf()); // pure ASCII: buf.byteLength == json.length const buf = new TextEncoder().encode(json).buffer as ArrayBuffer; const parseSpy = vi.spyOn(JSON, 'parse'); try { + // maxJsonBufferBytes == json.length: the byte check passes (buf.byteLength <= limit), + // but the decoded-string guard fires because text.length * 2 == json.length * 2 > json.length. + // JSON.parse must never be reached. try { - // maxJsonBufferBytes set to one less than the payload length — both the - // buffer-byte check and the decoded-string check should fire before - // JSON.parse is reached. - parseContainer(buf, { maxJsonBufferBytes: json.length - 1 }); + parseContainer(buf, { maxJsonBufferBytes: json.length }); } catch { - // expected to throw + // expected — the decoded-string guard fired } expect(parseSpy).not.toHaveBeenCalled(); } finally { parseSpy.mockRestore(); } - // At the exact payload length the parse should succeed. - expect(() => parseContainer(buf, { maxJsonBufferBytes: json.length })).not.toThrow(); + // With maxJsonBufferBytes == json.length * 2, both guards pass and parsing succeeds. + expect(() => parseContainer(buf, { maxJsonBufferBytes: json.length * 2 })).not.toThrow(); }); it('parses GLB container with JSON + BIN chunks', () => { @@ -1041,7 +1041,7 @@ describe('loadGltf', () => { const buffer = jsonToBuffer(minimalGltf()); await expect(loadGltf(buffer, { maxJsonBufferBytes: 1 })).rejects.toThrow(/payload too large/); - await expect(loadGltf(buffer, { maxJsonBufferBytes: buffer.byteLength })).resolves.toMatchObject({ + await expect(loadGltf(buffer, { maxJsonBufferBytes: buffer.byteLength * 2 })).resolves.toMatchObject({ meshes: [], nodes: [], }); From 8ac1510c6fad59156c093264ce37523d377b27a9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 14:26:48 +0000 Subject: [PATCH 14/26] Initial plan From 1d85646cad0e50fbab4ff6c0f44c72ba5c837d70 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 14:34:58 +0000 Subject: [PATCH 15/26] fix(csp): enforce strict-dynamic with hash; replace bare self with none where no scripts are loaded Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com> --- SECURITY.md | 65 +++++++++++++++++++++++++++++++++++++++++++++++ demo.html | 4 +-- demos.html | 2 +- gallery.html | 2 +- index.html | 2 +- tests/csp.test.ts | 19 ++++++++++++++ 6 files changed, 89 insertions(+), 5 deletions(-) create mode 100644 SECURITY.md diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..0a7fd9f --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,65 @@ +# Security Policy + +## Content Security Policy + +All public HTML pages in this project set a `Content-Security-Policy` meta tag. +The policy is intentionally strict and follows these principles: + +### `script-src` — no bare `'self'` + +Allowing `script-src 'self'` permits any script file served from the same origin +to execute, including any accidentally committed debug script or a compromised +static asset. Instead, each page uses the minimum required permission: + +| Page | Approach | Rationale | +|------|----------|-----------| +| `index.html` | `'none'` | No script is loaded | +| `demos.html` | `'none'` | No script is loaded | +| `gallery.html` | `'none'` | No script is loaded | +| `demo.html` | SHA-256 hash + `'strict-dynamic'` | Restricts execution to the single known inline bootstrap import | + +#### Hash-based policy for `demo.html` + +`demo.html` boots the application with a single inline module: + +```html + +``` + +The SHA-256 hash of that exact inline content (`import '/src/main.ts'`) is +pre-computed and embedded in the `script-src` directive: + +``` +script-src 'sha256-NDWEjzGVmgdl6gIijt3W2YpACKUzjdbNjuRCLQIRDKo=' 'strict-dynamic' +``` + +`'strict-dynamic'` propagates the trust granted by the hash to any modules +dynamically imported by that script (i.e. the rest of the application bundle). +Older browsers that do not understand `'strict-dynamic'` ignore it and fall back +to the hash alone. + +#### Recomputing the hash + +If the inline bootstrap script ever changes, recompute the hash with: + +```bash +printf "import '/src/main.ts'" | openssl dgst -sha256 -binary | base64 +``` + +Replace the `sha256-…` value in `demo.html` with the new output. + +### Other directives + +| Directive | Value | Reason | +|-----------|-------|--------| +| `object-src` | `'none'` | Disables Flash and other plug-in content | +| `base-uri` | `'self'` | Prevents base-tag hijacking | +| `default-src` | `'self'` | Safe fallback for unlisted resource types | +| `connect-src` | `'self' ws://localhost:* …` | Permits Vite HMR WebSocket in development | +| `unsafe-eval` | absent | Disallows `eval()` and similar constructs | + +## Reporting a Vulnerability + +If you discover a security vulnerability in this project, please open a GitHub +issue with the label **security**. For sensitive reports you may contact the +maintainers directly via the repository's GitHub profile. diff --git a/demo.html b/demo.html index 2b533bd..a8174c5 100644 --- a/demo.html +++ b/demo.html @@ -4,7 +4,7 @@ + content="default-src 'self'; script-src 'sha256-NDWEjzGVmgdl6gIijt3W2YpACKUzjdbNjuRCLQIRDKo=' 'strict-dynamic'; style-src 'self'; img-src 'self' data:; connect-src 'self' ws://localhost:* wss://localhost:* ws://127.0.0.1:* wss://127.0.0.1:*; object-src 'none'; base-uri 'self';" /> - + diff --git a/demos.html b/demos.html index 9eadcb7..45d52ba 100644 --- a/demos.html +++ b/demos.html @@ -4,7 +4,7 @@ + content="default-src 'self'; script-src 'none'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; connect-src 'self' ws://localhost:* wss://localhost:* ws://127.0.0.1:* wss://127.0.0.1:*; object-src 'none'; base-uri 'self';" /> + content="default-src 'self'; script-src 'none'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; connect-src 'self' ws://localhost:* wss://localhost:* ws://127.0.0.1:* wss://127.0.0.1:*; object-src 'none'; base-uri 'self';" /> + content="default-src 'self'; script-src 'none'; style-src 'self'; img-src 'self' data:; connect-src 'self' ws://localhost:* wss://localhost:* ws://127.0.0.1:* wss://127.0.0.1:*; object-src 'none'; base-uri 'self';" /> { expect(csp).toContain("base-uri 'self'"); } }); + + it("does not use a bare 'self' in script-src without a complementary nonce or hash", () => { + for (const path of pagePaths) { + const html = readFileSync(new URL(path, import.meta.url), 'utf8'); + const csp = extractCspContent(html); + if (csp === null) throw new Error(`${path} must have a CSP meta tag`); + const scriptSrcMatch = csp.match(/script-src\s+([^;]+)/); + if (scriptSrcMatch === null) continue; + const scriptSrc = scriptSrcMatch[1]; + if (scriptSrc.includes("'self'")) { + const hasNonce = /'nonce-[^']+'/.test(scriptSrc); + const hasHash = /'sha(?:256|384|512)-[^']+'/.test(scriptSrc); + expect( + hasNonce || hasHash, + `${path} uses bare 'self' in script-src without a nonce or hash`, + ).toBe(true); + } + } + }); }); From db64e8057f2ca17e17983691fd2346f5506d5da5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 14:55:24 +0000 Subject: [PATCH 16/26] fix(csp): clarify strict-dynamic requirement in SECURITY.md; add hash-drift test for demo.html Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com> --- SECURITY.md | 8 ++++++-- tests/csp.test.ts | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index 0a7fd9f..b0d4e0c 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -35,8 +35,12 @@ script-src 'sha256-NDWEjzGVmgdl6gIijt3W2YpACKUzjdbNjuRCLQIRDKo=' 'strict-dynamic `'strict-dynamic'` propagates the trust granted by the hash to any modules dynamically imported by that script (i.e. the rest of the application bundle). -Older browsers that do not understand `'strict-dynamic'` ignore it and fall back -to the hash alone. +**`demo.html` requires `'strict-dynamic'` support to function correctly.** +Without it, the browser will allow the inline bootstrap script (whose hash +matches) but block its `import '/src/main.ts'` call, because no host source +such as `'self'` is present to authorize the external module load. All browsers +that support WebGL 2.0 also support `'strict-dynamic'` (Chrome 52+, Firefox 52+, +Safari 15.4+), so this is not a practical limitation for this project. #### Recomputing the hash diff --git a/tests/csp.test.ts b/tests/csp.test.ts index 5b8f3d3..865cd5f 100644 --- a/tests/csp.test.ts +++ b/tests/csp.test.ts @@ -1,3 +1,4 @@ +import { createHash } from 'node:crypto'; import { readFileSync } from 'node:fs'; import { describe, expect, it } from 'vitest'; @@ -65,4 +66,21 @@ describe('Content-Security-Policy', () => { } } }); + + it('CSP hash in demo.html matches the actual inline bootstrap script content', () => { + const demoPath = new URL('../demo.html', import.meta.url); + const html = readFileSync(demoPath, 'utf8'); + const csp = extractCspContent(html); + if (csp === null) throw new Error('demo.html must have a CSP meta tag'); + + const scriptMatch = html.match(/]*type="module"[^>]*>([\s\S]*?)<\/script>/); + expect(scriptMatch, 'demo.html must contain an inline