eclipse / lemminx

XML Language Server
Eclipse Public License 2.0
267 stars 91 forks source link

possible cache escape #1566

Open jukzi opened 1 year ago

jukzi commented 1 year ago

https://github.com/eclipse/lemminx/blame/2b0fe29f52c111be3d5c1dccdf9d08340c63be31/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/uriresolver/CacheResourcesManager.java#L316 checking for ".." only does not prevent against more (tripple) dots on for example win95 https://cwe.mitre.org/data/definitions/32.html

angelozerr commented 1 year ago

@jukzi have you a concrete sample where there could be have a problem?

jukzi commented 1 year ago
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE root_element SYSTEM "http://anyhost/.../.../escapedthecachefolder">
<body>
</body>

image

jukzi commented 1 year ago

Please also note, that somehow <!DOCTYPE root_element SYSTEM "file://localhost:/secret"> tries to access c:/secret - why?? image

And it happens even if in eclipse IDE the option"download external resources like referenced DTD, XSD" is disabled! image

angelozerr commented 1 year ago

<!DOCTYPE root_element SYSTEM "file://localhost:/secret"> tries to access c:/secret - why??

The problem requires to be investigated. Do you think you could be interested to contribute with your issue?

And it happens even if in eclipse IDE the option"download external resources like referenced DTD, XSD" is disabled!

It should work since long time. After chaning the preference if you type a space in the XML, you should see:

image

jukzi commented 1 year ago

It doesn't matter what the dialog box says when it still tries to read arbitrary files from filesystem. I don't know how to debug lemmix. it's on another process then eclipse.exe. For the triple dot thing it would be enough to just search for "../" instead of "/../"

angelozerr commented 1 year ago

We are busy and I fear that we will have not time to do that for now.

I know that we have several tests about this check. If you feel to fix it in lemminx (+test) it should be really nice.

angelozerr commented 1 year ago

It doesn't matter what the dialog box says when it still tries to read arbitrary files from filesystem.

Ho the problem comes from when you use localhost? If you try http, https, is it working?

jukzi commented 1 year ago

I see the problem only for "file://localhost:". http/https or any host other then localhost does not show the effect.

angelozerr commented 1 year ago

Ok I understand more, if it starts with file we consider that it not a remote file and we try to download something.

Any contribution are welcome!

If you are interested to work on this issue and you need some help, please ask me.