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

Advanced XSS [⭐] #1245

Closed
2 tasks
bkimminich opened this issue Oct 28, 2019 · 8 comments
Closed
2 tasks

Advanced XSS [⭐] #1245

bkimminich opened this issue Oct 28, 2019 · 8 comments

Comments

@bkimminich
Copy link
Member

bkimminich commented Oct 28, 2019

⭐ Challenge idea

  • XSS into <script> context (with escaping for wrong context, e.g. HTML)
  • XSS into HTML attribute context

Underlying vulnerability/ies

XSS

Expected difficulty

⭐⭐ to ⭐⭐⭐⭐

@Scar26
Copy link
Contributor

Scar26 commented Feb 4, 2020

How about a CSP injection + onerror attribute XSS challenge? Something along the lines of:

the user is allowed to post images and provide the alt-text in case they don't render correctly. The CSP disables inline scripts but the image link provided by the user is added to it.

The rendered element then looks like :-
<img src="user.com/image.png" alt="user-defined alt text">

while the CSP looks something like :-
default-src 'self'; img-src "user.com/image.png";

This functionality is implemented so that the user can use a payload like alt" onerror="myscript in the alt-text to escape the alt attribute and inject an onerror script. They can then use the image link to inject into the CSP and allow inline scripts with a payload like user.com/image.png; script-src 'unsafe-inline' and achieve XSS.

Should I go ahead with this?

@Scar26
Copy link
Contributor

Scar26 commented Feb 6, 2020

@bkimminich Can you please give your opinion on the above mentioned idea so I know if/how I should proceed with this?

@bkimminich
Copy link
Member Author

I like the idea of covering HTML attribute breakout like this! The question is, will Angular allow this to happen "naturally"? Is this maybe easier to get into the legacy profile page which is based on Pug? If we go for the profile page, maybe the alt-attribute could just be the chosen username? That might be fun, because then that field would be usable to two different XSS paths, the name itself and the icon depending on the payload.

@Scar26
Copy link
Contributor

Scar26 commented Feb 6, 2020

Ah, yes!
I was thinking of going with a manual implementation in js to allow for this injection but this sounds better! Think I'll go with this

@Scar26
Copy link
Contributor

Scar26 commented Feb 14, 2020

Ok so after playing around with the profile page for a while, here's what I found.

Pug automatically escapes all attribute values making attribute breakout impossible by default. Also profile images are currently stored locally as files which rules out CSP injection as well.

So as far as I can see right now, there's two ways this challenge can be implemented:

  • Integrate CSP bypass in the the existing xss via username challenge and upgrade it to a higher difficulty

    • Add a CSP to the profile page so the inline xss doesn't work by default even after one gets through the regex filter
    • Add a third way to upload profile images, that is via any general url and update the user model to include this as a new property. If the user chooses to add an image via this method, it will be loaded directly from the url, which is also added to the CSP therefore making injection possible to allow the execution of the inline script as I described earlier
    • The bad thing about this approach is that
  • Disable escaping on the alt-text attribute

    • By doing this, we could implement the attribute breakout + CSP injection challenge that I initially pitched but intentionally disabling escaping for one specific attribute would take away much of the realism from this problem

@bkimminich , @J12934 Which alternative do you suggest I proceed with?

@Scar26
Copy link
Contributor

Scar26 commented Feb 15, 2020

@bkimminich Any opinions on this ^ ?

I'm thinking of going ahead with the first one, since we already have a few more low difficulty XSS challenges besides this one, so upgrading it to a higher difficulty won't hurt

@bkimminich
Copy link
Member Author

First option sounds better to me too!

@github-actions
Copy link

This thread has been automatically locked because it has not had recent activity after it was closed. 🔒 Please open a new issue for regressions or related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Aug 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants