mastodon / mastodon

Your self-hosted, globally interconnected microblogging community
https://joinmastodon.org
GNU Affero General Public License v3.0
47.28k stars 7.01k forks source link

Need defensive measures against Unicode direction overrides #1095

Closed zackw closed 6 years ago

zackw commented 7 years ago

If you put U+202E RIGHT-TO-LEFT OVERRIDE in your display name, the effects spill out of the display name field and affect other parts of the UI, as shown here:

screen shot of the bug described above

There are probably a bunch of other closely-related bugs.

(Inspired by a similar bug in Twitter found by @0xabad1dea.)


wxcafe commented 7 years ago

IMO this is not a bug, it's (at worse) funny. I've marked it enhancement for now (because the @ name should not be reversed, at least), feel free to say if you think it should considered a bug.

zackw commented 7 years ago

I do think this is a bug. Not the simple case I showed, and not necessarily with the display name, but I strongly suspect there are much worse things you can do with unusual Unicode constructs. Please research the history of UI breaks with untrusted Unicode.

Elizafox commented 7 years ago

The fact this is considered a feature and not a bug is worrying. It shows injection vulnerabilities aren't taken seriously in the name of "comedy."

I for one do not find that very funny.

wxcafe commented 7 years ago

... this is not an injection? It's a perfectly valid Unicode character that does what it's supposed to do and has no harmful repercussions on the users?

awilfox commented 7 years ago

Directional overrides are necessary for some names. That is definitely something that should never be filtered.

Simple fuzzing with Unicode strings (valid and invalid) should be all that is necessary to prove there is either a security bug or no bug. I suggest putting that in the test suite.

zackw commented 7 years ago

Directional overrides should not be filtered, but they should be contained. For instance, a directional override within the display name should not be able to change the presentation of anything other than the display name.

wxcafe commented 7 years ago

is it really that big a problem?

zackw commented 7 years ago

The specific test case I presented may not appear to be a big problem, but if you look into the history of this kind of bug, and consider all of the Unicode characters with nonlocal effects and all of the contexts where user-submitted arbitrary Unicode can appear within Mastodon's UI, you will realize that this is indeed a significant problem.

gandaro commented 6 years ago

This can be fixed by surrounding user-provided text by <bdi> tags, like this:

<bdi>&#x202E;Feli</bdi> says “hello”.

or specifying the dir attribute:

<span dir="auto">&#x202E;Feli</span> says “hello”.