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

Bootstrap v4 breaks Content-Security-Policy compared to Bootstrap v3 #25394

Closed
yahesh opened this issue Jan 22, 2018 · 53 comments · Fixed by #32759
Closed

Bootstrap v4 breaks Content-Security-Policy compared to Bootstrap v3 #25394

yahesh opened this issue Jan 22, 2018 · 53 comments · Fixed by #32759

Comments

@yahesh
Copy link

yahesh commented Jan 22, 2018

As it seems, Bootstrap v4 is now using "data:image/svg+xml" background-urls which leads to errors when using a Content-Security-Policy like default-src 'self'; form-action 'self'; frame-ancestors 'self'; require-sri-for script style. In order to be able to migrate from Bootstrap v3 to Bootstrap v4 one would have to weaken the Content-Security-Policy protection.

IMHO that's a regression.

broken CSP in chrome

@mdo
Copy link
Member

mdo commented Jan 22, 2018

Hmm, interesting—hadn't considered this at all with the embedded SVGs. I suppose the easiest path forward for this is documenting a way to swag our navbar toggler for a custom one.

@ogonkov
Copy link
Contributor

ogonkov commented Jan 24, 2018

Also, inline SVG put aside document parsing (more CSS to parse === more time document wait for it to be loaded)

I think it's better to inline them on project bundling via Grunt or Webpack (if you really want less http requests but bigger CSS bundle)

@cramhead

This comment has been minimized.

@XhmikosR

This comment has been minimized.

@cramhead

This comment has been minimized.

@XhmikosR

This comment has been minimized.

@cramhead

This comment has been minimized.

@XhmikosR

This comment has been minimized.

@cramhead

This comment has been minimized.

@yahesh

This comment has been minimized.

@mdo
Copy link
Member

mdo commented Sep 17, 2018

We'll try to tackle this as part of v5 when we get to that and can make breaking changes to our components to move the background images from base64 to in-HTML elements.

@mdo mdo closed this as completed Sep 17, 2018
@mjgs
Copy link

mjgs commented Dec 3, 2018

@mdo Bootstrap v5 seems like it could be a long way off. You mentioned earlier in the thread that a good workaround might be to replace the navbar toggler icon with a custom one. I tried doing this but the alignment was all messed up on small screens, could you share the documentation you had in mind, so those of us trying to implement CSP can get something functional and secure setup?

@ogonkov
Copy link
Contributor

ogonkov commented Dec 4, 2018

Could it be fixed with file-loader?

@XhmikosR
Copy link
Member

XhmikosR commented Dec 4, 2018

Note that we don't use one SVG, so this isn't only about the navbar toggler icon.

Your best bet is to relax CSP allowing data: for now.

@mjgs
Copy link

mjgs commented Dec 4, 2018

Note that we don't use one SVG, so this isn't only about the navbar toggler icon.

It's probably where most people building sites using bootstrap will run into this though since there are a lot of sites using navbars that are accessed from mobile devices.

Your best bet is to relax CSP allowing data: for now.

"Make your site insecure for an undetermined amount of time" - I can understand that doing a whole release for this issue is too big a task, but Bootstrap 5 could be several years away, and asking people to make their sites insecure is quite a big ask. I was hoping for a workaround in the interim, @mdo's suggestion sounded ideal. I don't agree that making my site insecure is a good idea.

@XhmikosR
Copy link
Member

XhmikosR commented Dec 4, 2018

Unfortunately, that's how it is. We can't introduce potentially breaking changes right now.

v5 is the next major version and work has already started in the v4-without-jquery branch.

@mjgs
Copy link

mjgs commented Dec 4, 2018 via email

@XhmikosR
Copy link
Member

XhmikosR commented Dec 4, 2018

Yeah, but like I said there are a lot more cases we use CSS-inlined SVGs, so I'm not sure if changing the toggler icon alone will be enough. I usually use purgecss in my build process so it's enough for me.

@mjgs
Copy link

mjgs commented Dec 4, 2018

It will at least be enough for fixing the navbar toggler csp issue. Using the octicon hamburger icon...

When I swap out

<span class="navbar-toggler-icon"></span>

for

<span data-toggle="tooltip"
      data-delay="<%= tooltipDelayMs %>"
      data-placement="bottom"
      title="Menu">
  <%- include('../icons/three-bars.svg') %>
</span>

The toggler renders fine in the browser and without a CSP error, although it looks a little different (slightly smaller), but the main problem is that when viewed in Chrome devtools device tool, it's set in from the right side for most phone sized devices.

How do I get it to render correctly on all screens?

@emiranda04
Copy link

@mdo "We'll try to tackle this as part of v5".
Amazing answer 👎
is BS3 vulnerable ? no rush to upgrade() : migrate to BS4()
Then you realize that BS4 breaks CSP.

@philwolstenholme
Copy link

philwolstenholme commented Dec 10, 2018

For people building Bootstrap from the source (rather than including the compiled CSS and minified JS) you could use https://www.npmjs.com/package/patch-package to edit (then subsequently patch on each install) the Bootstrap source to remove the data URIs and include the SVGs in an alternative way that will satisfy your CSP requirements.

@emiranda04
Copy link

Thanks for this info! I will look into it :-)

@mjgs
Copy link

mjgs commented Dec 10, 2018

The rendering issue looks like it might be a Chrome devtools bug, I have checked using a responsive design testing browser extension and all device sizes render correctly. I was also able to extract the bootstrap svg and use it instead of the octicon svg.

Here are the steps I took to get the toggler rendering without the CSP error:

  1. Download the bootstrap hamburger svg using devtools
  2. Add the svg to the HTML
<span class="hamburger" 
      data-toggle="tooltip"
      data-delay="<%= tooltipDelayMs %>"
      data-placement="bottom"
      title="Menu">
  <%- include('../icons/hamburger.svg') %>
</span>
  1. Add css
.hamburger {
  display: inline-block;
  height: 1.5em;
  vertical-align: middle;
  width: 1.5em;
}

@philwolstenholme

This comment has been minimized.

@Grinnz
Copy link

Grinnz commented Dec 20, 2018

I have not found any reason that it is actually a problem to include data: only in your img-src policy to allow this. But obviously don't allow it in script-src or default-src. See https://security.stackexchange.com/a/167244

@ffoodd
Copy link
Member

ffoodd commented Jun 30, 2020

We have open issues and PR about using inlined SVG (in the markup) instead of embedded in CSS. v5 is in it's first alpha, there'll be more breaking changes until a first stable release so please wait — or contribute.

But this issue is definetely in our v5 roadmap.

@yahesh
Copy link
Author

yahesh commented Jun 30, 2020

@ffoodd Thanks for clarification.

@crustamet
Copy link

Inline SVG would do it, i don't know why they don't implement this already ? if they have a hamburger or other svg inlines they should reallly create some files that can be replaced by other icons also.

@xi
Copy link
Contributor

xi commented Sep 1, 2020

@ffoodd I would be happy to contribute. Can you link the PR you mentioned? I could not find it.

@ffoodd
Copy link
Member

ffoodd commented Sep 1, 2020

There's no dedicated PR AFAIK, but multiple discussions on related issues and PR. You may start a dedicated PR if you want, just take some time to search and read comments here and there about inline SVGs.

@mdo
Copy link
Member

mdo commented Sep 14, 2020

After going back and forth with the team on this, we're most likely sticking with the embedded SVGs in v4 and in v5. We can't use SVGs in the HTML for things like our checkboxes, radios, and selects, so we'd have two different implementations for how we handle SVGs across the project (embedded CSS and inline HTML). It'd also be a larger departure from v4 than I'm willing to make just yet. v6 could be where we really overhaul some of this stuff though and make some different leaps.

Providing documentation on how to replace the images is definitely doable though. I'll stub out a PR shortly for that, along with guidance on what SVGs are included (most are from Bootstrap Icons, so they're easy to replace with external images).

Beyond that, anything else I'm missing?

@nickalbrecht
Copy link

nickalbrecht commented Sep 17, 2020

Maybe I'm misunderstanding. Wasn't the problem that enforcing a reasonable CSP policy of 'self', broke any element that uses inline style, inline script, or embedded images (data:) in the HTML. But that using Embedded images in the CSS itself was fine (since you can use SRI to trust the Bootstrap CSS/JS files? So the solution would be to embedded SVG's in the CSS, and never inject it into HTML?

Also, ensuring that anything that needs to hide/show something doesn't use inline styles, and instead sets a CSS class.

@gladiola
Copy link

gladiola commented Dec 1, 2020

I had this same problem with Boostrap Carousel colliding with Content Security Policy. I know it's not an answer to the problem, but I was able to do a workaround with the following simple steps.

  • Draft replacement left and right arrow icons in a drawing program. I used Gimp and built 100x100 pixel PNGs with transparent background. Place these graphics in a folder readily accessible under the Content Security Policy.
  • Drill down into bootstrap.css and identify the classes that need to be overridden.
  • Install replacement CSS, calling the PNG icons with background-image url().
  • Adjust the size of the receiving CSS class to meet the icons, as desired. I found it helpful to completely shut off the background attribute of one of the class calls.

`.carousel-control-prev-icon {
background-image: url("LeftArrow.png");
}

.carousel-control-next-icon {
background-image: url("RightArrow.png");
}

.carousel-control-prev-icon,
.carousel-control-next-icon {
display: inline-block;
width: 100px;
height: 100px;

}
`
These changes were near lines 6500 of my copy of bootstrap.css.

Notice that with this technique for a workaround, the color of the icons needs to be predetermined. The "fill" attribute of an SVG element won't be available.

The cold reality is that, when choosing between XSS protection and keeping SVGs for Bootstrap, it was time to throw the SVG problem overboard and secure the application. We simply can't have a design stylesheet breaking a security control that helps shut down an OWASP Top 10 problem. We hope that the Bootstrap team will be able to find a solution that works with CSP. Thanks.

@mdo
Copy link
Member

mdo commented Jan 11, 2021

Finally opened #32759 to suggest some copy around this. Please review and let me know what you think!

@9mido
Copy link

9mido commented Feb 17, 2021

For anybody who wants to see the results of that documentation PR here is the online version:

https://getbootstrap.com/docs/5.0/customize/overview/#csps-and-embedded-svgs

@mdo @XhmikosR and @ffoodd

What I am confused with this documentation PR is that there is no reference to URLs that you can plug into your CSP configurations or where in the CSP configurations to put them. You are just linking to the code documentation page for each element?

I was hoping bootstrap could provide something like this:

https://content-security-policy.com/examples/

https://stripe.com/docs/security/guide#content-security-policy

https://developers.google.com/recaptcha/docs/faq#im-using-content-security-policy-csp-on-my-website.-how-can-i-configure-it-to-work-with-recaptcha

Using URLs and plugging them in is much easier than locally hosted assets, inline images, etc.

I wouldn't know where to begin on where to actually download the images/svgs from and then figure out where to put them locally and then how to reference them in my code. There is no guidance for that.

@ffoodd
Copy link
Member

ffoodd commented Feb 17, 2021

@9mido Would you mind opening a new issue, please? Your suggestion makes sense and we can improve our docs, for sure.

jeremy-jameson added a commit to technology-toolbox/website that referenced this issue Nov 16, 2021
This is required by custom components (e.g. the "Tags" partial that renders tags with various font sizes and weights) as well as Bootstrap itself:

twbs/bootstrap#25394
jeremy-jameson added a commit to technology-toolbox/website that referenced this issue Nov 16, 2021
This is required by custom components (e.g. the "Tags" partial that renders tags with various font sizes and weights) as well as Bootstrap itself:

twbs/bootstrap#25394
jeremy-jameson added a commit to technology-toolbox/website that referenced this issue Nov 16, 2021
This is required by custom components (e.g. the "Tags" partial that renders tags with various font sizes and weights) as well as Bootstrap itself:

twbs/bootstrap#25394
charlesroelli pushed a commit to charlesroelli/bootstrap that referenced this issue Feb 7, 2022
charlesroelli pushed a commit to charlesroelli/bootstrap that referenced this issue Mar 1, 2022
@Harleym7000
Copy link

Harleym7000 commented Jul 14, 2022

An easy but slightly inconvenient fix for this would be to download the required SVG from the bootstrap icons website

Then change your navbar HTML to point to the local SVG instead. My example is to get the navbar toggle icon to display correctly on smaller-screen devices (mobiles and tablets)

I am saving the SVG to a folder named img

Change this:
<button class="navbar-toggler" type="button" data-bs-toggle="collapse" data-bs-target="#navbarSupportedContent" aria-controls="navbarSupportedContent" aria-expanded="false" aria-label="Toggle navigation"> <span class="navbar-toggler-icon"></span> </button>

To this:
<button class="navbar-toggler" type="button" data-bs-toggle="collapse" data-bs-target="#navbarSupportedContent" aria-controls="navbarSupportedContent" aria-expanded="false" aria-label="Toggle navigation"> <img src="/img/list.svg"> </button>

Here is the result:
image

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

Successfully merging a pull request may close this issue.