search

daveaglick / FluentBootstrap

Provides extensions, helper classes, model binding, and other goodies to help you use the Bootstrap CSS framework from .NET code.
http://www.fluentbootstrap.com
MIT License
200 stars 76 forks source link

Html encode all user supplied strings to prevent XSS #53

Closed zivillian closed 8 years ago

zivillian commented 8 years ago

encode all strings which are passed to this library via TagExtensions.AddContent and ContentExtensions.Content to prevent unexpected XSS vulnerability.

daveaglick commented 8 years ago

Thanks for this! I'm away from a computer for a couple days, but I'll review it as soon as I get back.

daveaglick commented 8 years ago

I know this is still pending - sorry about that. I'm shooting to get this merged and to generate a new release tomorrow or this weekend.

daveaglick commented 8 years ago

I took a look at the changes, but I'm not sure I totally understand the intent. FluentBootstrap is used in the Razor template to generate HTML so it shouldn't ever be getting strings directly from the user. I'm certainly sensitive to XSS and other vulnerabilities, so I'm curious what sort of attack vector you had in mind here?

My main concern is to continue to provide a way of inputting raw HTML into the content of an element if we need to. Am I correct that passing in new HtmlString("dont_escape_this") or Html.Raw("also_not_escaped") to the Content() method will still allow setting raw HTML as the content of an element? It looks that way from the tests you added (thanks for tests, BTW!) but I just want to be sure.

zivillian commented 8 years ago

The point is, that we are using your library to render comments which where previously supplied by the user and saved to DB. I did expect the following statement to do html escaping as razor would do:

B.Div().AddCss("comment-body").SetText(commentFromDB)

in razor this would be:

<div class="comment-body>@commendFromDB</div>

You are correct, that it's still possible to use plain html. If the developer wants to (and hopefully knows what she's doing) one could still use:

B.Div().AddCss("comment-body").AddContent(new HtmlString(commentFromDB))

which equals

<div class="comment-body>@Html.Raw(commendFromDB)</div>
daveaglick commented 8 years ago

Okay, cool. That makes sense, especially in a migration scenario where you previously had Razor @ directives and were expecting FluentBootstrap to work the same way. This will end up being a breaking change, so I'll need to make sure that gets well communicated.

Thanks!

daveaglick commented 8 years ago

New release 3.3.5.3 is out and up on NuGet.