1629506 - (CVE-2020-12409) U+2800 (Braille blank) is displayed as a blank; treat as a space and encode

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[Anonymous]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:75.0) Gecko/20100101 Firefox/75.0

Steps to reproduce:

invisible charaters such as "space" are usually converted into code when they come after the URL for example, http://google.com/%20%20%20%20%20%20%20%20%20%20%20%20%20%20index.html

There are some characters such as “⠀” (U+2800) which are not converted into code when added after the URL which may lead to URL spoofing using RTL characters since this area is really a messy area.

Actual results:

Shown as a blank space

Expected results:

Shoud be converted into code.

Comment 1

[Daniel Veditz [:dveditz]]

We do usually try to show space-equivalent characters as their code, but not other IDN characters. Is this one not marked as a space in the Unicode tables? We've had problems, especially on Mac fonts, where legit, but unsupported on mac, characters are displayed as blank instead of an "unknown" marker.

Comment 2

[Daniel Veditz [:dveditz]]

That particular character is a Braille blank, but it's not treated as a space character in Unicode. We should treat it that way and escape it in URLs as we do with other space-like characters.

Chrome's equivalent bug (same reporter) is https://bugs.chromium.org/p/chromium/issues/detail?id=1068531
their patch is in their networking code: https://chromium.googlesource.com/chromium/src.git/+/1114475dd86b5c2fae002a26edc44941fc4d3179

Looks like they will be shipping this fix in Chrome 83 which is currently in Beta. Originally scheduled for Jun 9, since they are skipping 82 entirely it might be earlier.

If you look at the chromium test file there are a lot of blank and problematic characters that they don't unescape for the URLbar display, that we do, not just this one character. I had trouble figuring out where we do that these days. We'll unescape one space, but multiple spaces get escaped. NBSP always gets escaped. Most utf8 is left alone. Found a routine that looked like it was checking the IDNblacklist, but most of those get unescaped too (in the path). Maybe we're using ICU? I'm not sure I trust that.

Here's the chromium testcase: https://chromium.googlesource.com/chromium/src.git/+/refs/heads/master/net/base/escape_unittest.cc
list in code: https://chromium.googlesource.com/chromium/src.git/+/refs/heads/master/net/base/escape.cc#196

Gijs: any ideas?

Comment 3

[Jonathan Kew [:jfkthame]]

Note that AFAICS any braille characters in the domain name would always result in punycode. This is only about the rest of the URL -- the path, query, etc. So it only has the potential for spoofing the site's address when used in conjunction with RTL characters that result in bidi reordering, so that it's unclear what is the domain and what is the path.

I think the real issue we should be addressing here is what bug 1623888 discusses: because we hide the scheme when it is "http://", if there are no strong directional characters in the domain (e.g. just a numeric IP address), then RTL characters in the path can inappropriately flip the display around so that it's confusing which part of the URL is in fact the domain. (We still highlight it as darker text, but that's a subtle clue people may not be aware of.)

IMO if we fix that, either by not hiding the http:// scheme (which would then provide the necessary LTR context to make an all-neutral domain stay in place) or by imposing LTR directionality on such a domain in some other way, the presence of "space"-like Braille characters in the rest of the path would be pretty innocuous.

Comment 4

[:Gijs (he/him)]

Some of our unescaping is https://searchfox.org/mozilla-central/rev/41c3ea3ee8eab9ce7b82932257cb80b703cbba67/intl/uconv/nsTextToSubURI.cpp#104 . But I've forgotten where the multiple space thing lives and I cannot find it, or the bug where we added it. Marco?

Comment 5

[:Gijs (he/him)]

Gah, finally found it - https://searchfox.org/mozilla-central/rev/41c3ea3ee8eab9ce7b82932257cb80b703cbba67/browser/components/urlbar/UrlbarInput.jsm#2425-2432 .

We can fairly easily add things there. I don't know how we got the current list though, so leaving ni on Marco for that.

Comment 6

[Marco Bonardo [:mak]]

That list is there from many years and predates me, along years more cases were added to it, but if you want to know the initial source of that list, I can't tell much about that. It has been added in bug 412458, I suspect it was generating comparing decodeURI and decodeURIComponent.
There's definitely confusion between losslessDecodeURI, UnEscapeURIForUI, decodeURI, decodeURIComponent that is result of many hands and years on the codebase, and lack of documentation about best practices.

Comment 7

[Marco Bonardo [:mak]]

I'll try to complete the work in Bug 1623888, that is delayed due to the many things on the team's plate. That bug looks like a good prevention for this problem based on comment 3. Of course we can also add this specific char to the list of "whitespace-like" ones in losslessdecodeuri.

Comment 8

[Marco Bonardo [:mak]]

Attachment: Bug 1629506. r=jfkthame

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for perma failures on browser_copying.js:
https://hg.mozilla.org/integration/autoland/rev/1cd445272ca5c15d154ee81ace153b3f1d637cb0

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&revision=456b217c30307a1393636f15028ca20902aa96be
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=299672999&repo=autoland

[task 2020-04-27T22:46:37.134Z] 22:46:37 INFO - TEST-PASS | browser/components/urlbar/tests/browser/browser_copying.js | Clipboard has the given value: 'http://example.com/a%20test' -
[task 2020-04-27T22:46:37.134Z] 22:46:37 INFO - Loading : http://example.com/a%E3%80%80test
[task 2020-04-27T22:46:37.134Z] 22:46:37 INFO -
[task 2020-04-27T22:46:37.135Z] 22:46:37 INFO - Buffered messages finished
[task 2020-04-27T22:46:37.135Z] 22:46:37 INFO - TEST-UNEXPECTED-FAIL | browser/components/urlbar/tests/browser/browser_copying.js | url bar value set - Got example.com/a%E3%80%80test, expected example.com/a　test
[task 2020-04-27T22:46:37.136Z] 22:46:37 INFO - Stack trace:
[task 2020-04-27T22:46:37.136Z] 22:46:37 INFO - chrome://mochikit/content/browser-test.js:test_is:1297
[task 2020-04-27T22:46:37.136Z] 22:46:37 INFO - chrome://mochitests/content/browser/browser/components/urlbar/tests/browser/browser_copying.js:doCheck:262
[task 2020-04-27T22:46:37.136Z] 22:46:37 INFO - promise callbackchrome://mochitests/content/browser/browser/components/urlbar/tests/browser/browser_copying.js:loadURL:340
[task 2020-04-27T22:46:37.136Z] 22:46:37 INFO - chrome://mochitests/content/browser/browser/components/urlbar/tests/browser/browser_copying.js:runTest:275
[task 2020-04-27T22:46:37.136Z] 22:46:37 INFO - chrome://mochitests/content/browser/browser/components/urlbar/tests/browser/browser_copying.js:nextTest:254
[task 2020-04-27T22:46:37.137Z] 22:46:37 INFO - promise callbackchrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForClipboard:1124
[task 2020-04-27T22:46:37.137Z] 22:46:37 INFO - chrome://mochikit/content/browser-test.js:test_waitForClipboard:1373
[task 2020-04-27T22:46:37.137Z] 22:46:37 INFO - chrome://mochitests/content/browser/browser/components/urlbar/tests/browser/browser_copying.js:testCopy:324
[task 2020-04-27T22:46:37.137Z] 22:46:37 INFO - chrome://mochitests/content/browser/browser/components/urlbar/tests/browser/browser_copying.js:doCheck:265
[task 2020-04-27T22:46:37.137Z] 22:46:37 INFO - promise callbackchrome://mochitests/content/browser/browser/components/urlbar/tests/browser/browser_copying.js:loadURL:340
[task 2020-04-27T22:46:37.137Z] 22:46:37 INFO - chrome://mochitests/content/browser/browser/components/urlbar/tests/browser/browser_copying.js:runTest:275
[task 2020-04-27T22:46:37.137Z] 22:46:37 INFO - chrome://mochitests/content/browser/browser/components/urlbar/tests/browser/browser_copying.js:nextTest:254
[task 2020-04-27T22:46:37.137Z] 22:46:37 INFO - promise callbackchrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForClipboard:1124
[task 2020-04-27T22:46:37.137Z] 22:46:37 INFO - chrome://mochikit/content/browser-test.js:test_waitForClipboard:1373

[...]

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/autoland/rev/578d8d0d8c6e202078409f99846c70a5e07b54a8
https://hg.mozilla.org/mozilla-central/rev/578d8d0d8c6e

Comment 12

[Daniel Bodea [:danibodea]]

I am attempting to perform the verification of this fix, but I am not sure I can reproduce it correctly. The exact same result is displayed on all the main channels (both the affected and fixed versions) and on both LTR and RTL builds, which is:

1. Open browser.

2. Copy this link to clipboard, which has 2 spaces inside the URL (before and after the 2 unicode space characters %20%20) :

   http://google.com/ %20%20 index.html

3. Paste the link into the address bar.
   Observe: The link is still displayed in the address bar, as seen in the step above.

4. Press Enter key.
   Observe: The link is now displayed as:

   http://google.com/%20%20%20 index.html

The first space is being transformed into unicode "%20", but the second space character, from before the index word, is not.

I cannot verify this fix if I cannot reproduce it. Please provide a clear set of steps to reproduce.
Thank you!

Comment 13

[Marco Bonardo [:mak]]

To reproduce this bug you need a url like this http://google.com/%20%20%20%20%20%20%20%20%20%20%20%20%E2%A0%80index.html where there is the \u2800 or E2%A0%80 char, then you should easily see that before the change that char is shown as an invisible char, while after the fix it's shown as E2%A0%80

The %20 spaces are unrelated to this fix, and are just an example to explain that making a url with many \u2800 chars could be abused to generate a spoofing url, for example http://google.com/%E2%A0%80%E2%A0%80%E2%A0%80%E2%A0%80%E2%A0%80%E2%A0%80%E2%A0%80%E2%A0%80%E2%A0%80%E2%A0%80index.html

Comment 14

[Anonymous]

Attachment: Confirmation.png

Comment 12 can be reproduced in this way too; looks like the first space isn't encoded.

Comment 15

[Marco Bonardo [:mak]]

We only encode \u0020 that are followed by another \u0020, to preserve some readability. But that's not what this bug is about.
This bug is about \u2800

Comment 16

[Emil Ghitta, QA [:emilghitta]]

I have managed to reproduce this issue using Firefox 77.0a1 (BuildId:20200410213700) on Windows 10 64bit.

This issue is verified fixed using Firefox 77.0b7 (BuildId:20200515125749) on Windows 10 64bit, macOS 10.14 and Ubuntu 18.04 64bit.

Comment 17

[Frederik Braun [:freddy]]

Attachment: advisory.txt

Comment 18

[Daniel Veditz [:dveditz]]

Bounty combined with bug 1623888 since that is required to make this work.