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

whiteList fails when using slashes to separate tag attributes (PR included) #268

Open
hensleysecurity opened this issue Dec 27, 2022 · 0 comments · May be fixed by #269
Open

whiteList fails when using slashes to separate tag attributes (PR included) #268

hensleysecurity opened this issue Dec 27, 2022 · 0 comments · May be fixed by #269

Comments

@hensleysecurity
Copy link

hensleysecurity commented Dec 27, 2022

Let's say you have whitelisted the img tag. The following will not get filtered (good):

<img src="cat.jpg"/>

And neither will this (good):

<img
src="cat.jpg"/>

However, this will get filtered (bad):

<img/src="cat.jpg"/>

The use of / as a separator is supported by browsers so this ought to work. As reported in this article, the following characters may be used to separate attributes in an HTML tag:

  • Space (0x20)
  • Slash (0x2F)
  • Carriage-Return (0x0D)
  • Line-Feed (0x0A)
  • Horizontal Tab (0x09)
  • Form-Feed (0x0C)

The problem seems to be that the regexes in spaceIndex() and parseAttr() do not know about slashes:

var reg = /\s|\n|\t/;

js-xss/lib/parser.js

Lines 169 to 170 in c339c1f

if (/\s|\n|\t/.test(c)) {
html = html.replace(/\s|\n|\t/g, " ");

Therefore, getTagName() should return img, but incorrectly returns img/src="cat.jpg" instead (which is obviously not on the whitelist). The attribute parser has the same issue: it comes back with all the attributes in one string separated by /.

The regexes in the code snippets above are doubly redundant, because \n (literal newline) and \t (literal tab) will already get matched by \s (any whitespace character). All of the other whitespace characters in the list above will also get matched by \s.

I can provide a PR that will fix the issue.

@hensleysecurity hensleysecurity linked a pull request Dec 28, 2022 that will close this issue
@hensleysecurity hensleysecurity changed the title whiteList fails when using slashes to separate tag attributes (one-line fix included) whiteList fails when using slashes to separate tag attributes (PR included) Jan 4, 2023
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.

1 participant