-
Notifications
You must be signed in to change notification settings - Fork 188
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
Switch to using a safe xml parsing library #186
Comments
I'm not familiar with But why would you worry about the parsing if the HTML is yours? |
@peterbe if you were to send an email containing user input, it could include xml couldn't it? I don't think I actually checked to see if that was sanitized. |
Indeed but if you generate the HTML (the one whose CSS is in a stylesheet and not in style tag attributes) and allow user input to be dangerously included without escaping, then that should probably be first thing to worry about. But I wouldn't mind an additional extra option to really tighten the lxml stuff too. It could start as an optional option. It's unlikely though, at this point, that I will work on that. |
@peterbe fair enough. Worth noting that defusedxml is a plug n' play replacement for lxml with literally no code changes required and no difference in input/output in my experience w/ it. |
The default lxml library has several exploitable issues within it. Primarily the concern here would be DoS attacks like the billion laughs attack. Thankfully, it looks like you've disabled external entity resolution. It would be pretty easy just to plug a library like defusedxml in to replace lxml and would provide a little bit of extra defense.
The text was updated successfully, but these errors were encountered: