helmetjs / helmet

Help secure Express apps with various HTTP headers
https://helmetjs.github.io/
MIT License
10.18k stars 367 forks source link

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

Closed EvanHahn closed 4 months ago

EvanHahn commented 4 months ago

getDefaultDirectives returns a shallow copy:

https://github.com/helmetjs/helmet/blob/6475da1139677ff4b9da71d4dc7cb58d3c9aef54/middlewares/content-security-policy/index.ts#L71

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 commented 4 months ago

Two things to note:

EvanHahn commented 4 months ago

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 commented 4 months ago

Done in d9319b801c3c2dfef3ab23bdd29f1de99c94e95b/#465.