Sanitize nicknames more carefully #313
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ricochet's UI does not make any network requests under any circumstances; if one happens, it's likely a bug and potentially an input sanitization issue that could lead to deanonymization. Using QML's network access manager factory, intercept all of these requests, trigger an assert, and make sure it's absolutely not possible for any network traffic to occur as a result. Contributes to ricochet-im#303, inspired by Sarah Jamie Lewis
QML makes the questionable decision of parsing all Text items as rich text if they appear to like HTML. This HTML parser is, fortunately, nothing like a browser - it supports only simple visual formatting. Still, in the worst case it can trigger network requests. We were being too lax with sanitizing variables before display, if they weren't directly from an external source. In one case, this could lead to deanonymization if a user reviews and accepts a contact request with a nickname containing an <img> tag. This is one of three commits aimed at preventing these issues in the future; the others more carefully block all network requests, and reduce the set of characters valid in nicknames. Fixes ricochet-im#274, discovered by Sarah Jamie Lewis
|
👍 |
|
Awesome! 👍 |
special
added a commit
that referenced
this pull request
Dec 23, 2015
Sanitize nicknames more carefully
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These commits harden against this problem in three ways: by blocking any possible network requests, ensuring nicknames are sanitized every time they're displayed, and by blocking a few more characters in incoming contact requests.
Nicknames in Ricochet are always set by the local user, so this wasn't directly a vulnerability. However, inbound contact requests can suggest a nickname, which is displayed (safely) and used if the user accepts the request without changing it. If that request was accepted despite the nickname, it would be possible for a remote user to trigger network activity.