-
Notifications
You must be signed in to change notification settings - Fork 84
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
By default, RazorEngine will not encode values to be HTML safe #65
Comments
I think at least RunHtmlSafe should be included in this library. (also Aysnc stuff). Is a PR accepted for that? |
I did not include
Could you please share what are you going to use RazorEngine for? |
We're creating HTML and render it to a pdf. Also in the PDF you could add javascript. Javascript is now disabled in the PDF, but we really prefer multiple lines of defense (LoD)
Of the RazorEngineExtensions for https://github.com/adoconnection/RazorEngineCore/wiki/@Raw - but maybe that isn't really a good solution.
I don't really see how I could RazorEngineCore.Extensions for that, as I want this to be safe:
where
Good one, unfortunately almost every example of razor is in combination with MVC and so Html Encoding. E.g. https://docs.microsoft.com/en-us/aspnet/core/mvc/views/razor?view=aspnetcore-5.0#expression-encoding |
Ah thx. I checked also the code today and I doubt if this works for strong typed compiled models. But will test that. Hopefully before the weekend :) |
Don't panic :)First of all, I have updated @Raw wiki it now contains code on how to make RazorEngine 100% HTML safe with few lines of code. The long answerIf you ever had a chance to work with classic ASP, you know there was a pain: devs had to encode literally every their output <%= Server.HTMLEncode(myVariable) %> it was very unhandy since in web you have to encode any user content. Skipping webforms, fast forward to ASP.NET Razordecades later you still have to encode every user content in web environment, so they decided to make razor html safe by default. Because encoding your output is primary usecase. And occasionaly you need to write unsafe html content where Detached RazorEngine has no primary usecaseIt can be used in
So as you see HTML safety does not applies everywhere. Think this way: What if RazorEngine were to render LaTeX templates and values there have to be encoded in completely different way. I dont think it has to be included as default feature. So the HTML safety. |
Amazing, thanks @adoconnection! I think your approach here is reasonable and I understand your reasoning for keeping the core job of this framework focused. I would suggest though that you highlight the default behaviour really clearly on the README as it's so easy for people to not understand or forget about html encoding and if the default behaviour leads them down the "wrong" track when generating HTML, it'll just be too easy for folks who aren't paying attention to end up in a bad place. Maybe it's literally a section titled "HTML generation and security" that's got some warning bells on it or something :P And just links to the @Raw docs. Gonna migrate our RazorEngine project over today to RazorEngineCore and see how I go! Thanks again :) |
Great answer!
I think also html encoding should be enabled by default if you could disable it. It's one of the secure design principles: https://en.m.wikipedia.org/wiki/Secure_by_default
|
I have updated readme to avoid confusion. I would argue. Default installation of MongoDB will let anyone on the internet to connect to it. ELK does not have HTTPS by default. Rediculous! But im trying to say that there are things not intended to use by newcomers. ASP.NET on the other hand is intended for use by rookies. One can not make secure angle grinder. New users will still chop their fingers, and others will be annoyed. |
@adoconnection , I seem to have a problem after following the layout + html safe templating docs whereby the It's like it's encoding the Edit:
Like this, when Hope that helps someone else. Almost wonder if it's worth adding this into the docs. |
@benmccallum we have the same issue here. We fixed it by not using template rendering in the layout page except @renderbody (and guaranteed it with units tests to be safe) I think your solution could work, it would be great to have it & document it 👍 |
this issue seems to be resolved |
Documented by https://github.com/adoconnection/RazorEngineCore/wiki/Package-comparison Seems good to me :) |
From https://github.com/adoconnection/RazorEngineCore/wiki/@Raw
I think this is a security issue. It's really unexpected and I'm sure many won't see this warning at all.
Please change the default in the next major version
The text was updated successfully, but these errors were encountered: