Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Content-Security-Policy getDefaultDirectives should return a deep copy #463

Closed
EvanHahn opened this issue Apr 27, 2024 · 3 comments
Closed

Comments

@EvanHahn
Copy link
Member

getDefaultDirectives returns a shallow copy:

const getDefaultDirectives = () => ({ ...DEFAULT_DIRECTIVES });

Someone could mutate this object and cause chaos. We should:

  1. Update this function to do a simple deep clone. I wouldn't pull in a library for this; I would update this function to return a copy.
  2. Add a test to ensure that each call gives a separate copy. Try mutating one and verify that the change doesn't appear if you call getDefaultDirectives again.

This is technically a breaking change so it should be made against the v8.0.0 branch.

@sohrb
Copy link
Contributor

sohrb commented Apr 28, 2024

Two things to note:

  • I do not see how the original code allowed for unsafe mutation since the spread operator is returning a copy (all be it a shallow one).
  • the structuredClone API provided by Node does perform a deep copy but doesn't copy functions. Is that a deal breaker?

@EvanHahn
Copy link
Member Author

You could cause a problem if you mutate one of the arrays. For example:

const one = getDefaultDirectives();
one["script-src"].push("https://garbage.example");

const two = getDefaultDirectives();
console.log(two["script-src"]);
// => ["'self'", "https://garbage.example"]

EvanHahn pushed a commit that referenced this issue Apr 29, 2024
@EvanHahn
Copy link
Member Author

Done in d9319b8/#465.

EvanHahn pushed a commit that referenced this issue May 25, 2024
EvanHahn pushed a commit that referenced this issue Jun 1, 2024
EvanHahn pushed a commit that referenced this issue Sep 28, 2024
EvanHahn pushed a commit that referenced this issue Sep 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants