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 open redirect vulnerability during trailing slash redirect. #1956

Closed
wants to merge 2 commits into from
Closed

Fix open redirect vulnerability during trailing slash redirect. #1956

wants to merge 2 commits into from

Conversation

bharath6365
Copy link
Contributor

No description provided.

Copy link
Member

@boutell boutell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please explain how to reproduce the vulnerability. Apostrophe's slug validator should not be recording any double slashes.

@bharath6365
Copy link
Contributor Author

bharath6365 commented Jun 26, 2019

Steps to Reproduce

  1. Go to any website powered by Apostrophe. In this case I'm making use of the demo site http://dashboard.apostrophecmsdemo.org/

  2. Visit this URL http://dashboard.apostrophecmsdemo.org/%2f%2fgoogle.com%2f
    As you can see its url encoded. (%2f is decoded as /). Now there will be a redirect because apostrophe doesn't accept trailing slashes.

  3. Check the location header sent for the redirect.
    https://www.evernote.com/l/ApTvRWGFoRBArb2F71MkT0XYmjer-lKP1BQB/image.png

  4. The page actually redirects to google.com.

@boutell
Copy link
Member

boutell commented Jun 26, 2019

You're right. I think replacing any series of multiple slashes with a single slash would be sufficient and would have a lesser impact on innocent code that could be out there; what do you think? Can you see a way to exploit that?

@boutell boutell closed this in 863f24a Jun 26, 2019
@boutell
Copy link
Member

boutell commented Jun 26, 2019

I pushed a fix for this that has a lesser impact, it just replaces multiple-slash sequences with a single slash. I'm working on releasing that now. Thank you for the report.

@boutell
Copy link
Member

boutell commented Jun 27, 2019

This was npm published yesterday. Thanks again for the report.

@bharath6365
Copy link
Contributor Author

bharath6365 commented Jun 27, 2019 via email

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 this pull request may close these issues.

None yet

3 participants