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

CVE-2018-20583 - XSS Vulnerability #337

Closed
colinodell opened this Issue Dec 30, 2018 · 1 comment

Comments

Projects
None yet
1 participant
@colinodell
Copy link
Member

colinodell commented Dec 30, 2018

An XSS vulnerability (CVE-2018-20583) has been identified in the following versions of this library:

  • 0.15.6
  • 0.15.7
  • 0.16.0
  • 0.17.0
  • 0.17.1
  • 0.17.2
  • 0.17.3
  • 0.17.4
  • 0.17.5
  • 0.18.0

It allows unsafe URLs to be added to links.

The issue has been fixed in version 0.18.1. All users should upgrade to version 0.18.1 immediately. Additionally, if your application caches the resulting HTML, please purge and/or regenerate those caches.

Summary

Malicious users can bypass the "unsafe links" restrictions by inserting an encoded newline character (%0A) into the URL's protocol like so:

[Click me](javascri%0Apt:alert('XSS'))

Certain versions of this library would decode, then fail to re-encode, that newline, resulting in the following HTML:

<p><a href="javascri
pt:alert('XSS')">Click me</a></p>

Browsers ignore the newline and see javascript: instead of javascri + \n + pt, thus allowing the JS to execute when the link is clicked.

Impact

Specially-crafted Markdown links can be created which, when clicked, would execute JavaScript in the browser.

Setting the allow_unsafe_links option to false, as recommended in the security documentation, would not have prevented this behavior in the affected versions.

Details

The URL normalization process basically runs rawurlencode(rawurldecode($url)) to make sure that everything that everything is properly encoded. When we implemented #287 (via commit 7d91ca0), the regex on line 85 would fail to match newline characters because the s regex modifier was missing. As a result, if you fed it a URL containing %0a, it basically:

  1. Decodes the %0a to a newline character (\n) during the decode() step
  2. Fails to re-encode the newline back to %0a during the encode() step
  3. CommonMark ultimately outputs the href attribute with the \n newline in the middle of it
  4. The browser ignores the newline in the HTML attribute

Adding the s modifier to that regular expression fixes step 2 and ensures that all newlines in a URL are always properly encoded.

Credits

A huge thank you to Austin H. for finding the issue and working with @GrahamCampbell to responsibly disclose it!

@colinodell colinodell self-assigned this Dec 30, 2018

@colinodell colinodell pinned this issue Dec 30, 2018

@thephpleague thephpleague locked as resolved and limited conversation to collaborators Dec 30, 2018

@colinodell

This comment has been minimized.

Copy link
Member

colinodell commented Dec 30, 2018

This issue has been fixed in version 0.18.1. All users should upgrade to version 0.18.1 immediately.

Please refer to the revised description above (#337) for a full description of this issue.

@colinodell colinodell changed the title XSS Vulnerability - 2018-12-29 XSS Vulnerability - CVE-2018-20583 Dec 30, 2018

@colinodell colinodell changed the title XSS Vulnerability - CVE-2018-20583 CVE-2018-20583 - XSS Vulnerability Dec 30, 2018

@colinodell colinodell unpinned this issue Jan 1, 2019

@colinodell colinodell closed this Jan 15, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.