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

Fix potential XSS vulnerability in master remote login code display #3223

Merged
merged 2 commits into from Feb 5, 2018

Conversation

kke
Copy link
Contributor

@kke kke commented Jan 30, 2018

It's possible to inject javascript into the remote login HTML page.

Exploitability is quite small, the attacker would have to trick the user to go to something like master_url/code#code=abcd<script>alert("foo")</script>.

Since the master does not have any cookie authentication and the browser won't send any bearer auth headers, the attacker would seem to have very little to gain by doing so.

One possible attack could be to display a fake cloud login screen.

@kke kke added this to the 1.5.0 milestone Jan 30, 2018
@kke
Copy link
Contributor Author

kke commented Jan 30, 2018

ping @nevalla

@kke
Copy link
Contributor Author

kke commented Jan 30, 2018

image

Not sure if the regex is too harsh, maybe the code can contain other characters? Maybe just replace <?

@SpComb
Copy link
Contributor

SpComb commented Feb 2, 2018

I can't repro this, at least on firefox the window.location.hash is URL-encoded, so the resulting document.write(...) not actually HTML:

>>> window.location.hash
"#code=foo%3Cscript%3Ealert(%22foo%22)%3C/script%3Ebar"

EDIT: this seems specific to firefox, with chrome it ends up getting blocked by the XSS auditor?

@SpComb
Copy link
Contributor

SpComb commented Feb 2, 2018

Anyways, the right way to fix this XSS is to replace the document.write with an insintrically safe method like innerText = ...: https://www.owasp.org/index.php/DOM_based_XSS_Prevention_Cheat_Sheet#RULE_.237_-_Fixing_DOM_Cross-site_Scripting_Vulnerabilities

    <div id="code" style='...'></div>
    <script>
      var code = window.location.hash.split('code=')[1].split('&')[0];
      document.getElementById("code").innerText = code;
    </script>

Trying to regexp your way out of XSS injection usually just ends in tears.

@SpComb
Copy link
Contributor

SpComb commented Feb 5, 2018

👍 with innerText:

2018-02-05-113606_826x589_scrot

@kke kke merged commit 06a7a06 into master Feb 5, 2018
@kke kke deleted the fix/potential_xss_vulnerability_in_remote_login branch February 5, 2018 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants