Security: Global loading error list can grow unbounded (memory DoS risk)

[tuanaiseo]

Problem

The global Zustand store appends every loading error to errors and never prunes it. In long-lived sessions or attacker-driven repeated failed asset loads, this can cause unbounded memory growth and eventual UI instability.

Severity: low
File: src/core/Progress.tsx

Solution

Cap stored errors (e.g., keep last N entries), deduplicate repeated URLs, or periodically reset error state.

Changes

• src/core/Progress.tsx (modified)

Testing

• Existing tests pass
• Manual review completed
• No new warnings/errors introduced

[vercel]

@tuanaiseo is attempting to deploy a commit to the Poimandres Team on Vercel.

A member of the Team first needs to authorize it.

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

[travisbreaks]

The underlying issue is real (unbounded array growth in a long-lived session), but the framing and implementation could be stronger.

On the framing

Calling this a "security" issue and "memory DoS risk" overstates it. DefaultLoadingManager.onError only fires when Three.js fails to load an asset (texture, model, etc.). An attacker would need to cause thousands of asset load failures in a single browser session to create meaningful memory pressure. This is a resource hygiene improvement, not a security fix. I'd suggest retitling to something like "fix: cap unbounded error accumulation in Progress store."

On the implementation

The .slice(-100) approach works but has two issues:

1. Unnecessary allocation: [...state.errors, item].slice(-100) always creates the full N+1 array, then a second array of up to 100. A simple length check avoids the extra allocation when under the cap:

set((state) => ({
  errors: state.errors.length >= 100
    ? [...state.errors.slice(-99), item]
    : [...state.errors, item]
}))

2. No deduplication: The same URL failing repeatedly (e.g., a broken texture in a render loop) fills all 100 slots with identical entries. A URL-based dedup would be more useful:

set((state) => ({
  errors: state.errors.includes(item)
    ? state.errors
    : [...state.errors, item].slice(-100)
}))

Missing from the PR

• The cap (100) is hardcoded with no constant or configurability
• No tests added
• Strategies 2 and 3 from the description (dedup, periodic reset) were not implemented

Verdict

The fix is directionally correct but could be tighter. The dedup is arguably more important than the cap since a single broken asset in a render loop is the most realistic scenario.