search

fnando / svg_optimizer

Some SVG optimization based on Node's SVGO
MIT License
19 stars 6 forks source link

fix: do not set Nokogiri's `noent` parse option by default #16

Closed flavorjones closed 1 year ago

flavorjones commented 1 year ago
PR Checklist ### PR Structure - [x] This PR has reasonably narrow scope (if not, break it down into smaller PRs). - [x] This PR avoids mixing refactoring changes with feature changes (split into two PRs otherwise). - [x] This PR's title starts is concise and descriptive. ### Thoroughness - [x] This PR adds tests for the most critical parts of the new functionality or fixes. - [x] I've updated any docs, `.md` files, etc… affected by this change.

What

Allow users to opt into entity expansion on trusted documents by passing in the kwarg:

trusted: true

Otherwise do not enable this option, because it's not safe for untrusted documents.

Why

The fix introduced for #8 in b1b5013d introduces a security issue.

The Nokogiri documentation for ParseOptions::NOENT says:

NOENT Substitute entities. Off by default. ⚠ This option enables entity substitution, contrary to what the name implies. ⚠ It is UNSAFE to set this option when parsing untrusted documents.

Please feel free to contact me directly at mike.dalessio@gmail.com for more information about the security impact (I'm the Nokogiri maintainer).

Known limitations

N/A

fnando commented 1 year ago

Thank you for this pull request! I merged it via #17.

fnando commented 1 year ago

Just released v0.3.0 with this change. Thanks again!

flavorjones commented 1 year ago

:bow: Thank you!