Skip to content

fix: keep __proto__ node ids out of Object.prototype in the node map - #590

Open
rahul05ranjan wants to merge 1 commit into
digitalbazaar:mainfrom
rahul05ranjan:fix/proto-pollution-flatten
Open

rahul05ranjan wants to merge 1 commit into
digitalbazaar:mainfrom
rahul05ranjan:fix/proto-pollution-flatten

Conversation

@rahul05ranjan

Copy link
Copy Markdown

Fixes #584

Problem

jsonld.flatten() builds its node map with plain objects, so a subject whose
@id is __proto__ resolves to Object.prototype at the lookup
(subjects[name] = subjects[name] || {} in lib/nodeMap.js): its @id and
its properties are written onto Object.prototype and the subject itself is
silently dropped from the output. The same happens for a named graph named
__proto__, and mergeNodeMapGraphs' merged map has the same shape, so the
node is lost again on merge.

Reproduction on main (with the four npm deps stubbed, since this host has no
network installs) — the PoC from the issue:

delete Object.prototype['@id'];
delete Object.prototype['http://example.com/polluted'];
const doc = {'@context': {polluted: 'http://example.com/polluted'},
  '@id': '__proto__', polluted: 'yes'};
await jsonld.flatten(doc);
Object.prototype['@id'];  // "__proto__" -- written by the node map

Fix

The graphs container, each subject map and the merged node map are now created
with Object.create(null) (in nodeMap.js, in the two flatten call sites in
jsonld.js, and in frame.js's graph map, which feeds the same
createNodeMap). On a prototype-less map, __proto__ is stored as a plain own
entry, so the subject survives and nothing reaches Object.prototype.

Verification

  • The PoC above now returns
    [{'@id': '__proto__', 'http://example.com/polluted': [{'@value': 'yes'}]}]
    and leaves Object.prototype untouched; same for a named graph named
    __proto__.
  • Three mocha cases added to tests/misc.js (flatten with @id: __proto__,
    named graph __proto__, and jsonld.createNodeMap keeping the entry).
  • Equivalence check on 5 documents × flatten/createNodeMap/compact/expand plus
    a frame run (plain IRIs, blank nodes, named graphs, @list, a merge): output
    is byte-identical before and after, except the __proto__ node map entry,
    which was silently {} before and now holds the node.
  • The full mocha suite could not run on this host (no npm registry access);
    the library was exercised directly through require('.../lib/jsonld.js')
    with minimal stubs for canonicalize, lru-cache, rdf-canonize and
    @digitalbazaar/http-client.

createNodeMap stored subjects and graphs in plain objects, so a node
with @id '__proto__' (or a graph with that name) resolved to
Object.prototype: its @id and properties were written there and the
subject was silently lost. The graphs, their subject maps and the
merged node map are now created without a prototype, so such a name
is stored as its own entry.

Co-authored-by: rahul05ranjan <11793455+rahul05ranjan@users.noreply.github.com>
@rahul05ranjan

Copy link
Copy Markdown
Author

Hi! This PR has all checks passing and is ready for review. Would appreciate a look whenever you have a moment 🙏

@rahul05ranjan

Copy link
Copy Markdown
Author

Hey! Just wanted to follow up on this security fix for proto pollution in the node map. The branch is current with main and all looks green. Appreciate any time you can spare for a review. 🙏

@rahul05ranjan

Copy link
Copy Markdown
Author

Hi @digitalbazaar team — this PR is ready for review. All checks are green where available. Let me know if anything needs tweaking!

@rahul05ranjan

Copy link
Copy Markdown
Author

Ready for review when you have a moment!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prototype pollution in jsonld.flatten() when processing a node with @id __proto__

1 participant