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 XSS caused by invalid utf8. #11

Merged
merged 3 commits into from Feb 18, 2017
Merged

Conversation

evazion
Copy link
Contributor

@evazion evazion commented Feb 17, 2017

Here's a rather interesting bug I found with the the help of american fuzzy lop. Stick the following into a view and observe an XSS:

  <%= format_text("[[\xFA<script \xFA>alert(document.cookie); //\xFA</script \xFA>]]", :ragel => true) %>

@user\xF4<b>xss\xFA</b> and "url":/page\xF4">x\xFA<b>xss\xFA</b> are similar exploits.

The issue is that g_markup_escape_text is used for escaping html, but this function assumes the input is valid utf8. When given invalid utf8 it goes haywire. Specifically, \xFA is an invalid leading byte, which seems to cause the function to copy the next few bytes unescaped until it resynchronizes with the utf8 stream.

The fix I went with is to raise an exception if the input isn't valid utf8. I also removed some stuff that tries to handle nulls since g_validate_utf8 forbids those too. AFAICT the purpose of this code was to close any unclosed tags when EOF is reached, but that's redundant since parse_helper calls dstack_close anyway.

In practice, postgres doesn't allow text fields to contain invalid utf8 or nulls to begin with, so in the real world this doesn't seem possible to trigger.

This fixes an XSS in the use of `g_markup_escape_text`, which doesn't
properly escape input if it's not valid utf8.

Since nulls are also forbidden by `g_utf8_validate`, don't try to deal
with them either.
@r888888888 r888888888 merged commit 5a1d932 into r888888888:master Feb 18, 2017
@evazion evazion deleted the fix-utf8-xss branch February 18, 2017 05:02
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

2 participants