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

Stop recommending defusedxml instead of lxml / warn about specific lxml misuse #47

Open
xmo-odoo opened this issue Aug 23, 2022 · 0 comments

Comments

@xmo-odoo
Copy link

  • defusedxml.lxml was never production-grade, and is now deprecated, it has multiple incompatibilities with lxml "proper" and breaks code
  • lxml had options around sensitive XML features, and has added more after defused outlined issues:
    • resolve_entities defaults to True, probably is what allows quadratic blowup and local entity expansion attacks (that lxml is not sujbect to billion laughs I expect means it doesn't do recursive entity expansion), it might be a bit brutal to require it being disabled however it's an option and lxml's FAQ provides a recipe for restricted entity expansion
    • no_network defaults to True (no network lookup) and protects against external entity expansion and DTD retrieval, disabling it should probably be flagged
    • huge_tree protects against xml bombs by default, enabling it should probably be flagged
    • lxml's xinclude support is opt-in (also), use of these should probably be flagged

Finally xpath and xslt are a bit more complicated as they're "legit and safe" in the same sense e.g. database APIs are, running a "static" query should be safe (and lxml's xpath API supports parametrisation) but untrusted xpath and xslt injection / untrusted execution has similar issues to sql.

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

No branches or pull requests

1 participant