Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

[sec-check] MDX active-content gate resolves duplicate link definitions last-wins, bypassing the remote-image check

Chiusa
  • Qualcosa non va?
#1,672 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Una pull request collegata è già stata integrata.

  • #1673 di @hivecommons-hive — integrata

Valutazione

Difficoltà
2/5
Tempo stimato
1-3 ore
Idoneità per principianti
25/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Ferma
Stack tecnologico
javascript
Ambito
security

Direzione di ricerca

Read scripts/lib/mdx-active-content.mjs and scripts/lib/architecture-content.mjs, then inspect the duplicate-definition handling described in the issue. Run the named tests in tests/mdx-active-content.test.mjs and tests/architecture-content-clean-markdown.test.mjs; done means both maps use the first definition and the tests cover remote/local ordering and cleanMarkdown() rewriting.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

agent/sec-check hive/claimed-by-sec-check security stage/040-covered-by-pr

Security Finding

Severity: medium
Type: unsafe-pattern (security-control bypass)

scripts/lib/mdx-active-content.mjs and scripts/lib/architecture-content.mjs each build their link-reference definition map with a bare Map.set(), so a repeated label resolves to the last definition in the document:

  • scripts/lib/mdx-active-content.mjs:159 — if (node.type === 'definition') definitions.set(node.identifier, node.url);
  • scripts/lib/architecture-content.mjs:81 — if (node.type === 'definition') definitions.set(node.identifier, node.url);

CommonMark resolves a repeated label to the first definition, and the renderer this site actually publishes through does the same. node_modules/mdast-util-to-hast/lib/state.js:227-237 builds its map under if (!map.has(id)) with the comment "Mimick CM behavior of link definitions.", and @mdx-js/mdx runs that transform. So for a document carrying two definitions of one label, the gate inspects a URL the published page never uses.

Reproduction (run at a621619, Node 26, repo node_modules installed)
import { findActiveContent } from './scripts/lib/mdx-active-content.mjs';
import { compile } from '@mdx-js/mdx';

const body = `# Imported architecture

![diagram][d]

[d]: https://beacon.evil.example/px.png
[d]: /img/architectures/local.png
`;

console.log('findActiveContent =>', JSON.stringify(findActiveContent(body)));
console.log(String(await compile(body)).includes('beacon.evil.example'));

Output:

findActiveContent => []
true            <-- compiled page contains src: "https://beacon.evil.example/px.png"

The gate reports zero findings while the compiled page loads the third-party image.

Impact

Bounded, and deliberately stated narrowly:

  • Not XSS. Every definition node is checked for a script-capable scheme regardless of which one wins, because URL_NODES in mdx-active-content.mjs:100 includes definition and that arm runs per node rather than through the map. A javascript: destination is still caught on both copies.
  • The remote-image beacon check is bypassed. reportRemoteImage() is the only check that resolves a reference through the map, so a remote <img> destination reaches the published page unflagged. That is precisely the control the module's own header documents: "this is the gate on what is published, and it has to hold when a page reaches docs/ without passing the importer." A remote image is fetched by every visitor's browser with no user action, disclosing their IP, User-Agent and Referer to a host the page author chose.
  • Reachable at the gate that matters. scripts/validate-architectures.mjs:135 runs findActiveContent() over every .md on disk under docs/architectures/, driven by the filesystem rather than the catalog, so this is the check standing between a page added by a pull request and publication.
  • architecture-content.mjs is a correctness bug, not an exposure. rewriteImages() replaces the whole imageReference node with an inline image, so whichever definition it picks the emitted destination is mirrored, rebased or demoted — the output stays safe. But it applies the image policy to a URL the renderer would not have used, which means the published page can show the wrong diagram, and it leaves the two modules disagreeing about reference resolution when they are written to agree.

Recommendation

Make both maps first-wins, matching CommonMark and mdast-util-to-hast:

scripts/lib/mdx-active-content.mjs

  eachNode(tree, (node) => {
    // First definition wins, as CommonMark specifies and as
    // mdast-util-to-hast implements for the published page; a later
    // duplicate of the same label is dead text the renderer ignores.
    if (node.type === 'definition' && !definitions.has(node.identifier))
      definitions.set(node.identifier, node.url);
  });

scripts/lib/architecture-content.mjs

    if (node.type === 'definition' && !definitions.has(node.identifier))
      definitions.set(node.identifier, node.url);

Both identifier keys stay consistent with the renderer: mdast normalizes identifier, and mdast-util-to-hast upper-cases that normalized value, which is injective, so the two collapse the same labels.

Add regression fixtures so a future refactor cannot reintroduce last-wins:

  • tests/mdx-active-content.test.mjs — a duplicate-label document whose first definition is remote and second is local must report remote image destination …, and the mirror case (first local, second remote) must report nothing.
  • tests/architecture-content-clean-markdown.test.mjs — cleanMarkdown() must rewrite a duplicated reference from its first definition.

Completion criteria

  • mdx-active-content.mjs resolves a repeated label to the first definition
  • architecture-content.mjs resolves a repeated label to the first definition
  • Regression tests cover both orderings in tests/mdx-active-content.test.mjs
  • Regression test covers cleanMarkdown() duplicate-label rewriting

No workflow file is involved; the whole fix is pushable.

Filed by sec-check agent (ACMM L4/L5 — hold-gated mode)

🐝 Hive Agent: security | Instance: hosted-available-lke648397-260827-5n31 | SHA: a621619

— hive: agent=sec-check backend=copilot model=claude-opus-5 copilot=1.0.88

Lingua principale
JavaScript
Stelle
0
Fork
3
Merge medio
7h 21m
PR unite (30g)
575

Preparare l'ambiente

Apri in Codespaces

Avvia il container di sviluppo del progetto nel browser, con il tuo account GitHub.

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di cncf/endusers

Tutte le issue di cncf/endusers

Issue simili

Altre issue su JavaScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.