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.

Sign up for GitHub

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

Jump to bottom

Any specific reason to use NOENT? #55

Closed
shubhampathak opened this issue Dec 6, 2019 · 4 comments
Closed

Any specific reason to use NOENT? #55

shubhampathak opened this issue Dec 6, 2019 · 4 comments

Comments

@shubhampathak
Copy link
Contributor

@ckruse Hi Christian, found NOENT in both rbLibXMLParser.rb and rbNokogiriParser.rb
Passing NOENT as parsing option is risky.

Passing NOENT (which is used to substitute entities) as parsing options permits processing of entities, including both regular and external. That means NONET and NODTDLOAD will be of no use if NOENT is there.

Take a look at this:
sparklemotion/nokogiri#1582 (comment)

This is why Nokogiri team strictly suggests using default parsing options:
DEFAULT_XML = RECOVER | NONET

NOENT
@ckruse
Copy link
Owner

ckruse commented Dec 6, 2019

Hm. I actually don't remember anymore. It is several years ago that I wrote this code. Following the links you posted it seems to make sense to disable it. Do you mind to create a pull request?

@shubhampathak
Copy link
Contributor Author

Hi Christian, thanks for the quick response. Sure! creating a Pull Request.

shubhampathak added a commit to shubhampathak/CFPropertyList that referenced this issue Dec 6, 2019
@shubhampathak
refactor: remove NOENT option for security
b568969 
NOENT, which is used to substitute entities also enables processing of
both regular and external entities. Which seems risky when you don't
the XML source. NONET will have no effect if used with NOENT.

- Github Issue: ckruse#55
@shubhampathak shubhampathak mentioned this issue Dec 6, 2019
Merged
@ckruse
Copy link
Owner

ckruse commented Dec 6, 2019

fixed by #56

@ckruse ckruse closed this as completed Dec 6, 2019
@ckruse
Copy link
Owner

ckruse commented Dec 6, 2019

I pushed a new version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ckruse @shubhampathak