CNA-BUG-123: ContentManager has no in-progress guard, so an .xnb external-reference cycle recurses until the stack overflows

CNA snapshot 3b43bd86  ·  Known Issues › Current bugs  ·  source links pinned to 3b43bd86

Load<T> and LoadUntypedXnbReference cache an asset only after it has loaded, so an asset whose external reference leads back to itself re-enters the loader without bound and crashes instead of throwing ContentLoadException.

Identifier

CNA-BUG-123

Category

Bug

Subsystem

Content & XNB/CNB/CNJ

Status

Open

Verified against

CNA 3b43bd86 (3b43bd8687ed46523d873f9bec4b32bd8481dcb5)

Severity

Low (a triage suggestion, not a project priority)

Evidence basis

Source-verified: read at TARGET, not executed

Tests touching this area

Yes: see Current tests

Affected contract

ContentManager::Load<T>(const std::string&), ContentManager::LoadUntypedXnbReference, ContentReader::ReadExternalReference<T>() and ContentReader::ReadExternalReference()

Expected behaviour

CNA's content hardening refuses hostile file structure with a ContentLoadException instead of crashing: XnbReadLimits.hpp exists “to fail fast with a clear error”, and the ObjectDepthGuard comment in ContentReader.hpp calls unbounded recursion “driven entirely by attacker-controlled file structure” a bug class. A reference cycle between assets is the cross-file form of that recursion.

Actual behaviour at TARGET

Load<T> in ContentManager.hpp looks the key up in loadedAssets_ and stores the result only after LoadXnbAsset<T> returns; LoadUntypedXnbReference in ContentManagerUntyped.cpp does the same under its std::any key. Nothing marks an asset as being loaded. ReadExternalReference<T>() calls Load<T> and the untyped overload calls LoadUntypedXnbReference (the typed overload is defined in ContentManager.hpp, the untyped one in ContentManagerUntyped.cpp), so an EffectMaterial whose effect reference names its own asset, or an ExternalReference value naming an asset that refers back, re-enters the loader, re-reads and re-parses the file each time, and recurses until the stack is exhausted. The per-reader ObjectDepthGuard cannot stop it: each level is a fresh ContentReader starting at depth 0.

Source locations

• modules/content/include/Microsoft/Xna/Framework/Content/ContentManager.hpp — ContentManager::Load<T>: cache lookup, cache insert only after a successful load; ContentReader::ReadExternalReference<T>() re-enters it
• modules/content/src/Xna/ContentManagerUntyped.cpp — ContentManager::LoadUntypedXnbReference (same shape under the std::any key) and ContentReader::ReadExternalReference(), which re-enters it
• modules/content/src/Xnb/EffectMaterialContentTypeReaders.cpp — EffectMaterialReader::Read (effect external reference) and ExternalReferenceReader::Read

Evidence

Checked by reading at 3b43bd86; not executed. The XNA 4.0 reference behaves the same way: the IL of ContentManager.Load<T> checks loadedAssets and adds only after ReadAsset<T> returns, so genuine XNA would also overflow; its content pipeline cannot emit such a cycle, so only hand-edited, corrupted or hostile files reach it. It is listed as a bug because CNA's own robustness contract promises a clean refusal for hostile structure, not because XNA differs.

Focused reproduction

// Illustrative; not compiled or run. materialA.xnb is an EffectMaterial whose effect
// external reference is "materialB"; materialB.xnb references "materialA".
auto effect = content.Load<std::shared_ptr<Effect>>("materialA");
// Expected: ContentLoadException. Actual: A -> B -> A ... until the stack overflows.

Current tests

No test in modules/content/tests loads a reference cycle. ContentReaderExternalReferenceTests.cpp covers containment (absolute and escaping references are refused). XnbContainerFuzzTests.cpp mutates single files and cannot form a cross-file cycle.

Regression test

Two fixture files that reference each other, plus one that references itself, each expected to throw ContentLoadException. The fix is an in-progress set keyed like the cache (or a load-depth counter on the manager) consulted by both Load<T> and LoadUntypedXnbReference.

Blast radius

Only content with a reference cycle. Pipeline-built content, acyclic reference graphs and the .cnj/.cnb tiers are unaffected. The failure is a process crash, not an exception.

Workaround

Load content only from trusted sources.