Skip to content

fix(clone): copy a __proto__ member instead of assigning through the setter - #3175

Open
suraiya-ctrl wants to merge 1 commit into
Turfjs:masterfrom
suraiya-ctrl:clone-proto-member
Open

suraiya-ctrl wants to merge 1 commit into
Turfjs:masterfrom
suraiya-ctrl:clone-proto-member

Conversation

@suraiya-ctrl

Copy link
Copy Markdown

@turf/clone copies every input key with cloned[key] = value. When the GeoJSON came from JSON.parse and has a member literally named __proto__ (an ordinary own property on parsed objects), that assignment hits the Object.prototype.__proto__ setter on the fresh clone instead of copying the member:

  • cloneProperties: the returned properties object gets the input value as its prototype, so cloned.properties.x reads keys the original never exposed and Object.keys does not show
  • cloneFeature / cloneFeatureCollection: the foreign-member loop does the same, and the prototype is the very same object reference as the input, so the clone is not independent of it
  • the member itself is dropped, so clone(f) is no longer deeply equal to f

The effect stays on the returned object (the global Object.prototype is not touched), but any consumer that clones untrusted GeoJSON and then reads properties by name gets attacker-chosen values back. A __proto__ key is now defined with Object.defineProperty as a plain own property, the same way lodash's baseAssignValue handles it. Not a breaking change. The new tape test fails on master and passes with the fix.

  • Meaningful title, including the name of the package being modified.
  • Summary of the changes.
  • Heads up if this is a breaking change: it is not.
  • Any issues this resolves: none filed.
  • Inclusion of your details in the contributors field of package.json: left out to keep the diff to the fix, happy to add.
  • Confirmation you've read the steps for preparing a pull request.

This branch has not been deployed

No deployments
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.

1 participant