Skip to content

fix(security): add URI length cap in validateExternalUri to prevent ReDoS#357

Merged
ormidales merged 3 commits into1.1.14from
copilot/patch-fix-validate-external-uri
Mar 13, 2026
Merged

fix(security): add URI length cap in validateExternalUri to prevent ReDoS#357
ormidales merged 3 commits into1.1.14from
copilot/patch-fix-validate-external-uri

Conversation

Copy link
Contributor

Copilot AI commented Mar 13, 2026

  • Add MAX_URI_LENGTH = 2048 module-level constant in GltfLoader.ts
  • Enforce length check before strict-mode allowlist regex in validateExternalUri
  • Updated MAX_URI_LENGTH JSDoc to accurately describe the guard (resource exhaustion, not backtracking)
  • Strict-mode length rejection error now includes len=…, max=… for easier diagnostics
  • Add unit test: URI > 2048 chars rejected in strict mode
  • Add unit test: URI exactly 2048 chars accepted in strict mode
  • Add unit test: no length cap applied in non-strict mode
  • All 400 tests pass
Original prompt

This section details on the original issue you should resolve

<issue_title>[PATCH] validateExternalUri strict-mode regex susceptible to catastrophic backtracking on adversarial input</issue_title>
<issue_description>Category: security
Severity: patch
File(s): src/core/GltfLoader.ts (validateExternalUri)

Description

The strict-mode allowlist regex /^[A-Za-z0-9._\-/]+$/ is applied to both the raw URI and its decoded form via candidates.every(…). For typical inputs this is benign, but the regex engine on some JS runtimes exhibits non-linear backtracking when the input contains many alternating allowed/disallowed characters (e.g. a 10 000-character string of a.). Since validateExternalUri is called inside resolveBuffers for every buffer entry in a glTF asset, a malformed asset can trigger this in a loop.

Problematic code example

// src/core/GltfLoader.ts — validateExternalUri
if (strict && !candidates.every((v) => /^[A-Za-z0-9._\-/]+$/.test(v))) {
  throw new Error();
}

Suggested fix

Add an explicit length cap before running the regex to bound execution time regardless of input content.

const MAX_URI_LENGTH = 2048;
if (strict) {
  if (candidates.some((v) => v.length > MAX_URI_LENGTH)) {
    throw new Error(`Buffer ${bufferIndex}: URI exceeds maximum allowed length in strict mode.`);
  }
  if (!candidates.every((v) => /^[A-Za-z0-9._\-/]+$/.test(v))) {
    throw new Error();
  }
}

Acceptance criteria

  • A URI length cap (e.g. 2048 characters) is enforced before the strict-mode regex runs
  • A unit test confirms that a URI longer than the cap is rejected in strict mode
  • Non-strict mode is unaffected</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

… catastrophic backtracking

Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com>
Copilot AI changed the title [WIP] [PATCH] Fix regex backtracking issue in validateExternalUri fix(security): add URI length cap in validateExternalUri to prevent ReDoS Mar 13, 2026
Copilot AI requested a review from ormidales March 13, 2026 20:41
@ormidales ormidales added this to the v1.1.14 milestone Mar 13, 2026
@ormidales ormidales marked this pull request as ready for review March 13, 2026 20:44
Copilot AI review requested due to automatic review settings March 13, 2026 20:44
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a strict-mode URI length cap to glTF external buffer URI validation to prevent excessively long URIs from being accepted, with tests covering strict/non-strict boundary behavior.

Changes:

  • Introduce MAX_URI_LENGTH = 2048 and enforce it in validateExternalUri() when strict is enabled.
  • Add test coverage ensuring strict mode rejects >2048, allows ==2048, and non-strict mode does not apply the cap.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/core/GltfLoader.ts Enforces a strict-mode maximum URI length during external URI validation.
tests/gltf.test.ts Adds regression tests for strict-mode URI length cap and non-strict behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ormidales
Copy link
Owner

@copilot apply changes based on the comments in this thread

…error

Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 13, 2026

@copilot apply changes based on the comments in this thread

Both review comments addressed in ed64cb9:

  1. Updated the MAX_URI_LENGTH JSDoc to say "guards against excessively long URIs that could cause resource exhaustion" instead of the inaccurate "catastrophic backtracking" phrasing.
  2. The length rejection error now includes (len=…, max=…) for easier diagnostics:
    Buffer 0: URI exceeds maximum allowed length in strict mode (len=2049, max=2048).
    

Copilot AI requested a review from ormidales March 13, 2026 20:49
@ormidales ormidales merged commit acfd81a into 1.1.14 Mar 13, 2026
2 checks passed
@ormidales ormidales deleted the copilot/patch-fix-validate-external-uri branch March 13, 2026 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[PATCH] validateExternalUri strict-mode regex susceptible to catastrophic backtracking on adversarial input

3 participants