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

URL XSS issue #183

Closed
panec opened this issue May 25, 2020 · 3 comments · Fixed by #200
Closed

URL XSS issue #183

panec opened this issue May 25, 2020 · 3 comments · Fixed by #200
Assignees
Labels

Comments

@panec
Copy link
Contributor

panec commented May 25, 2020

In the xss_api.js there is a function for sanitize URL which does not allow for not correct URL to be rendered.

function sanitizeURL(url) {
  try {
    const decodedUrl = decodeURIComponent(url);

    if (XRegExp(RELATIVE_REF).test(decodedUrl) || XRegExp(URI).test(decodedUrl)) {
      return url;
    }
  } catch (e) {
    // ignore
  }
  return '';
}

What is is odd for me is the way it is constructed.

The following URL
https://via.placeholder.com/1280x550&text=desktop%201280x550
is marked as invalid.

The problematic part is an escaped space character %20. Because its escaped version looks like this:
https://via.placeholder.com/1280x550&text=desktop 1280x550
which is not a valid URL

To make this valid you need to double escape the space %20 -> %2520:
https://via.placeholder.com/1280x550&text=desktop%25201280x550

Why the test function decodes the url before checking if it is a valid URI?

@panec
Copy link
Contributor Author

panec commented May 25, 2020

Additional tiny improvement might be added by updating format_xss.js function of

function isUriAttribute(name) {
  return name === 'href' || name === 'src';
}

into

function isUriAttribute(name) {
  return name === 'href' || name === 'src' || name === 'srcset';
}

as I believe that srcset always needs to be an URI

@tripodsan tripodsan self-assigned this May 26, 2020
@tripodsan
Copy link
Contributor

as I believe that srcset always needs to be an URI

it's a bit more complicated: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/source#attr-srcset

tripodsan added a commit that referenced this issue Jun 30, 2020
tripodsan added a commit that referenced this issue Jun 30, 2020
adobe-bot pushed a commit that referenced this issue Jun 30, 2020
## [4.5.2](v4.5.1...v4.5.2) (2020-06-30)

### Bug Fixes

* **xss:** test xss against original form ([#200](#200)) ([56f6733](56f6733)), closes [#183](#183)
@adobe-bot
Copy link

🎉 This issue has been resolved in version 4.5.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging a pull request may close this issue.

3 participants