Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

this.fetch mutates headers passed to it #528

Closed
antony opened this issue Dec 5, 2018 · 2 comments
Closed

this.fetch mutates headers passed to it #528

antony opened this issue Dec 5, 2018 · 2 comments

Comments

@antony
Copy link
Member

antony commented Dec 5, 2018

Just spent a while hunting this down, but if you keep a reference to a set of request defaults, the mutation of opts.headers at the end of preload context's fetch method can cause a security issue whereby non-authenticated clients end up getting the cookies of an authenticated user's request (And seeing privileged data).

Essentially I think it's best not to mutate things that are passed into a method. There has been some effort not to do this, but headers is always nested in opts and therefore becomes a reference.

I also note that this is a potential security issue when dealing with headers which can store credentials, so I think it's worth fixing.

Line causing the issue is here:

@Conduitry
Copy link
Member

From working through this with @antony, my suggested but untested fix is to replace

if (!opts.headers) opts.headers = {};

with

opts.headers = Object.assign({}, opts.headers);

Rich-Harris added a commit that referenced this issue Feb 1, 2019
avoid mutating `opts.headers` in `this.fetch` (#528)
@Rich-Harris
Copy link
Member

released 0.25 with the fix

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants