Open TomNUSDS opened 2 years ago
When this gets fixed, I think it'd be a great idea to also include a CI check that a CSP wouldn't get violated by future changes.
@TomNUSDS I'm thinking of using this package https://www.npmjs.com/package/scrollbar-width to solve this issue, rather than having react-uswds implement its own width checker. I'm unfamiliar with CSPs, but I want react-uswds to be as compliant as possible. Will this little package suffice?
Thanks for looking into this issue and making security a priority!
I checked out that package and unfortunately, it seems to manipulate styles directly:
div1 = document.createElement('div');
div2 = document.createElement('div');
div1.style.width = div2.style.width = div1.style.height = div2.style.height = '100px';
div1.style.overflow = 'scroll';
div2.style.overflow = 'hidden';
The only solutions that are CSP compliant use class names, put the required styles into that css classes and set the element's className. This is a bit of a pain because it requires the css to be included with the build versus just a pure code approach.
Another possible solution for your Dialog
, is to pass in a default scrollbar width as an optional property and if it's set, then the component Dialog doesn't do the measurement. This would allow code that cares about CSP to implement their own safe of measuring it and pass it along.
If you're interested in learning how css style manipulation can be used as an attack vector, here's a good article on invicti.com.
Hi @TomNUSDS can you check https://github.com/trussworks/react-uswds/pull/2761 to see if it works for your CSP?
Your fix LGTM!
Thanks for the follow-up!
Hi, Is there an update on this? Looks like there is a pull request from February, but it's still sitting in open waiting for approval. The code still does x.setAttribute('style',''visibility: hidden; overflow: scroll; ms-overflow-style: scrollbar'')
, which violates CSP. https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/style-src
ReactUSWDS Version & USWDS Version:
ReactUSWDS: 2.7.2 USWDS: 2.13.0
Describe the bug A CSP (content-security-policy) is good requirement for government sites. See https://content-security-policy.com/
A starting point for a CSP includes
style-src 'self'
This means that using<element style=
is forbidden as well as a page having a<style>
header. Only class names and external stylesheets.To Reproduce Steps to reproduce the behavior:
Have a static build project (generates a static html site).
Add a CSP to the index.html as a
<meta>
(this is an alternative to modifying server headers)(DISCLAIMER: this is the minimal CSP to demonstrate this bug an actual CSP would include a lot more restrictions)
Add code to display a react-uswds Modal dialog
Load page with the Modal dialog Result: the debug console shows
Discussion
The Modal's
getScrollbarWidth()
function to measure the width of the scrollbar breaks this policy. SpecificallyThis should be a relatively easy fix using a class name (see: https://stackoverflow.com/questions/986937/how-can-i-get-the-browsers-scrollbar-sizes
mpen
answer which includes a link to David Walsh's blog:).Device and Browser Information (please complete the following information if describing a UI bug):