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

DocumentHelper.parseText could be vulnerable to XML Injection #28

Closed
s0m30ne opened this issue Aug 13, 2017 · 2 comments
Closed

DocumentHelper.parseText could be vulnerable to XML Injection #28

s0m30ne opened this issue Aug 13, 2017 · 2 comments
Assignees
Milestone

Comments

@s0m30ne
Copy link

s0m30ne commented Aug 13, 2017

The DocumentHelper.parseText could be used to convert Strings to a Document Object. But the function uses SAXReader to parse XML String which is vulnerable to XML Injection. To deal with this problem, we always using setFeature function to disllow doctype and entity. However you use the SAXReader directly and do not offer any function to allow users to config those features. So I think this is a problem here and if users used DocumentHelper.parse and the string to convert was controled by user input such as GET or POST parameters, the application would be vulnerable to XML Injection.

@s0m30ne s0m30ne changed the title DocumentHelper.parseText could be XML DocumentHelper.parseText could be vulnerable to XML Aug 13, 2017
@s0m30ne s0m30ne changed the title DocumentHelper.parseText could be vulnerable to XML DocumentHelper.parseText could be vulnerable to XML Injection Aug 13, 2017
@FilipJirsak FilipJirsak self-assigned this Aug 14, 2017
@FilipJirsak FilipJirsak added this to the 2.1.0 milestone Aug 14, 2017
@FilipJirsak
Copy link
Contributor

DocumentHelper.parseText() is only helper method, you should instantiate own SAXReader when you want to configure it. But you are right that helper method should be safe and if someone needs unsafe variant, they can use full SAXReader.
So following features are disabled now in DocumentHelper.parseText():

@superfofo
Copy link

this function can be merged into the version dom4j-2.0.2, so that the project which jdk version is under jdk8 can apply ?

FilipJirsak added a commit to FilipJirsak/dom4j that referenced this issue Mar 12, 2020
FilipJirsak added a commit to FilipJirsak/dom4j that referenced this issue Mar 12, 2020
FilipJirsak added a commit that referenced this issue Apr 11, 2020
FilipJirsak added a commit that referenced this issue Apr 11, 2020
FilipJirsak added a commit that referenced this issue Apr 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants