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

XML External Entity (XXE) Vulnerability #9

Open
Nadahar opened this issue Jul 30, 2018 · 3 comments
Open

XML External Entity (XXE) Vulnerability #9

Nadahar opened this issue Jul 30, 2018 · 3 comments

Comments

@Nadahar
Copy link

Nadahar commented Jul 30, 2018

I've found that cling is vulnerable to XML External Entity (XXE) Processing exploits. We're using Cling, and I've confirmed that we're vulnerable using evil-ssdp.

My first approach was to try to create a PR for Cling, but because of the tight integration with Android I had to give up building Cling. My second approach was to override vulnerable classes in Cling, and this seemed to go reasonably well until I hit LastChangeParser which extends SAXParser from Seamless.

I can't seem to override my way out of this, so I'm trying to see if anything can be done to protect SAXParser (and any other vulnerable parts of Seamless).¨

I would love to create a PR, the problem is that my knowledge of XML isn't great enough to properly understand what's happening here. I understand that the fundamental problem is when XML parsers accept external URLs for DTDs, schemas etc. The easy fix is to simply disallow processing of external entities and/or DTD in general. In a lot of XML parsing this isn't needed anyway, but looking at this code it looks like these things are potentially in use, which makes protection much more difficult. I guess some kind of whitelist could be a potential solution, and of course the ability to turn it off when it's not needed.

Anyway, I'm simply in over my head here. It seems to me like I'd have to learn a lot more about XML and XML parsing to be able to come up with a robust solution for solving this. I believe it would be a lot easier for somebody with better knowledge of this code and XML parsing in general.

Is this something you would consider looking into? "First aid" for protecting the most common Java XML parsers can be found here, and as long as external entity processing isn't needed, it's quite straight forward.

@christianbauer
Copy link
Member

I don't use or maintain Cling anymore. For this issue I would be willing to merge a pull request with a tested fix and do a new minor release. One of the many commercial users of Cling should have the budget to do this. I would assume the fix has to be done in https://github.com/4thline/seamless in the classes SAXParser and DOMParser.

Related: 4thline/cling#243

@christianbauer
Copy link
Member

thanks

@Sami32
Copy link

Sami32 commented Nov 3, 2018

This XXE security issue wasn't fixed:
UniversalMediaServer/UniversalMediaServer#1522 (comment)

This issue should be re-opened.

@christianbauer christianbauer reopened this Nov 6, 2018
courville added a commit to nova-video-player/seamless that referenced this issue May 5, 2020
courville added a commit to nova-video-player/seamless that referenced this issue May 5, 2023
…thlineGH-9""

This reverts commit 6613247.

This should not be necessary anymore by using in cling AndroidUpnpServiceConfiguration UDA10ServiceDescriptorBinderImpl instead of UDA10ServiceDescriptorBinderSAXImpl as advised in 4thline/cling#247 for a workaround.
This will maintain seamless codebase aligned with EOL git repo.
courville added a commit to nova-video-player/seamless that referenced this issue May 6, 2023
…acks - 4thlineGH-9"""

This reverts commit 29e6e56.

and revert again since we need sax to list files
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

3 participants