Last Comment Bug 479336 - (CVE-2009-0652) IDN blacklist needs to include box-drawing characters
(CVE-2009-0652)
: IDN blacklist needs to include box-drawing characters
Status: RESOLVED FIXED
[sg:low]
: fixed1.8.1.21, fixed1.9.1, verified1.9.0.9
Product: Core
Classification: Components
Component: Networking (show other bugs)
: Trunk
: All All
: P2 normal (vote)
: mozilla1.9.2a1
Assigned To: Daniel Veditz [:dveditz]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-02-19 16:50 PST by Daniel Veditz [:dveditz]
Modified: 2009-04-16 08:39 PDT (History)
13 users (show)
benjamin: blocking1.9.1+
dveditz: blocking1.9.0.9+
dveditz: wanted1.9.0.x+
dveditz: wanted1.8.1.x+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch - v1 (2.21 KB, patch)
2009-02-20 17:58 PST, Reed Loden [:reed] (use needinfo?)
dveditz: review+
benjamin: approval1.9.1+
dveditz: approval1.9.0.9+
dveditz: approval1.8.1.next+
Details | Diff | Splinter Review
1.8.0 version (1.85 KB, patch)
2009-04-09 07:47 PDT, Martin Stránský
no flags Details | Diff | Splinter Review

Description Daniel Veditz [:dveditz] 2009-02-19 16:50:06 PST
The IDN code contains a blacklist of characters that are confusingly similar to others that automatically kick us into the punycode display. This is stored as the pref "network.IDN.blacklist_chars".

Apparently we're missing the box-drawing characters, and a recent talk at blackhat DC appears to have taken advantage of that to spoof an SSL connection to a bank.
https://www.blackhat.com/presentations/bh-dc-09/Marlinspike/BlackHat-DC-09-Marlinspike-Defeating-SSL.pdf

Since it's just a screen shot I can't be sure, but I can reproduce the results with those characters.
Comment 1 Daniel Veditz [:dveditz] 2009-02-20 02:02:42 PST
Specifically I meant 
 2571 BOX DRAWING LIGHT DIAGONAL UPPER RIGHT TO LOWER LEFT

There are other potential omissions from the list, characters that look similar to other characters we've already included. Should maybe consider
 066A ARABIC PERCENT SIGN 
   (and maybe 0609 and 060A depending on font)
 2052 COMMERCIAL MINUS SIGN
both are percent signs with tiny dots, which on my system/font made a pretty good slash in the URL field.
 2041 CARET INSERTION POINT (looks like a slash with a smudge on the bottom)

If we've included 05C3 HEBREW PUNCTUATION SOF PASUQ because it looks like a colon, shouldn't we also have
 02D0 MODIFIER LETTER TRIANGULAR COLON
 0589 ARMENIAN FULL STOP
 2236 RATIO
 A789 MODIFIER LETTER COLON

We've got 0702 SYRIAC SUBLINEAR FULL STOP. We might want to look into
 0701 SYRIAC SUPRALINEAR FULL STOP    (I doubt this one)
 0703 SYRIAC SUPRALINEAR COLON
 0704 SYRIAC SUBLINEAR COLON

We're inconsistent about fractions. We block on xBC and xBD but not xBE, and in the range 2153-215F which are all fractions we reject seven and allow six.

One that doesn't work in my font but shows up in the Unicode charts as a slash-like char is
 1735 PHILIPPINE SINGLE PUNCTUATION

In any case, we need to add 2571.
Comment 2 Gervase Markham [:gerv] 2009-02-20 09:50:51 PST
Yep, chuck them all in. If we find a letter that has this problem, we may have to stop and ask more questions, but if it's punctuation, I have no problem adding it.

Gerv
Comment 3 Daniel Veditz [:dveditz] 2009-02-20 14:34:53 PST
Many of those punctuation characters probably would have been coalesced under NAMEPREP had they existed in earlier Unicode versions. Looks like the IETF is in the process of standardizing "IDNA2008" to replace the earlier IDNA2003 (rfc 3490,3491,3492) we're following.

I filed bug 479520 on looking into that for a future version, for now we should just blacklist these characters.

I didn't find any characters scarier than what Moxie Marlinspike revealed at BlackHat, unhiding this bug to reduce duplicate filings.
Comment 4 Reed Loden [:reed] (use needinfo?) 2009-02-20 17:58:21 PST
Created attachment 363436 [details] [diff] [review]
patch - v1
Comment 5 Daniel Veditz [:dveditz] 2009-02-23 22:22:49 PST
Comment on attachment 363436 [details] [diff] [review]
patch - v1

r=dveditz

This much looks good.
Comment 6 Reed Loden [:reed] (use needinfo?) 2009-02-24 08:52:12 PST
http://hg.mozilla.org/mozilla-central/rev/a884555d99c5
Comment 7 Daniel Veditz [:dveditz] 2009-02-25 15:40:57 PST
Comment on attachment 363436 [details] [diff] [review]
patch - v1

approved for 1.8.1.21 and 1.9.0.8, a=dveditz for release-drivers
Comment 8 Daniel Veditz [:dveditz] 2009-02-28 23:19:16 PST
Fix checked into the 1.8 branch
Comment 9 Daniel Veditz [:dveditz] 2009-02-28 23:22:58 PST
Fix checked into the 1.9.0 branch
Comment 10 Reed Loden [:reed] (use needinfo?) 2009-03-03 20:11:17 PST
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/4d2b36c2398a
Comment 11 Al Billings [:abillings] 2009-04-02 11:37:29 PDT
Verified that fix is checked in for 1.9.0.
Comment 12 Martin Stránský 2009-04-09 07:47:34 PDT
Created attachment 371866 [details] [diff] [review]
1.8.0 version

Note You need to log in before you can comment on or make changes to this bug.