Skip to content
This repository has been archived by the owner on Feb 15, 2023. It is now read-only.

Fix tag name filtering that could result in XSS #375

Closed
wants to merge 1 commit into from

Conversation

duvduvfb
Copy link

@duvduvfb duvduvfb commented Feb 6, 2017

gumbo_tag_from_original_text currently uses isspace to detect illegal whitespaces in tag names.

isspace will match on \v and \r, which are not illegal according to the spec (https://html.spec.whatwg.org/multipage/syntax.html#tag-name-state).

This can result in an XSS that will not be possible in a standard-compliant parser: In the current implementation, gumbo_tag_from_original_text will return <script> on the unknown element <script\v> (or <script\rnotreallyscript>).

Serializers relaying on gumbo_tag_from_original_text (such as prettyprint) will transform non-executable <script\v> tags to executable <script> tags.

This diff applies the whitelist in the spec to gumbo_tag_from_original_text and adds unit-tests for this scenario.

`gumbo_tag_from_original_text` currently uses `isspace` to detect illegal
whitespaces in tag names.

`isspace` will match on `\v` and `\r`, which are not illegal according to the spec
(https://html.spec.whatwg.org/multipage/syntax.html#tag-name-state).

This can result in an XSS that will not be possible in a standard-compliant
parser: In the current implementation, `gumbo_tag_from_original_text` will return
`script` on the unknown element `script\v` (or `script\r`).

Serializers relaying on `gumbo_tag_from_original_text` (such as `prettyprint`)
will transform non-executable `<script\v>` tags to executable `<script>` tags.

This diff applies the whitelist in the spec to `gumbo_tag_from_original_text`
and adds unit-tests for this scenario.
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@duvduvfb
Copy link
Author

Legal issues with CLA - can't sign it :(

@duvduvfb duvduvfb closed this Mar 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants