315411 - fail to check the IDN is in whitelist with un-normalized URL

Status: VERIFIED FIXED

(Core :: Networking, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

if user inputs un-normalized URL in URL bar, we always fail to check the IDN is in whitelist or not so. Becuase we are checking un-normalized host.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.0

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

e.g.,
http://www.日本語。ＪＰ/

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Sorry, it cannot repro. Please use this instead.
http://日本語。ＪＰ/

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

This can be reproduced when I hover the mouse cursor on comment 3's link. In this time, the status bar shows punycode URL.

Comment 5

[Darin Fisher]

So, this allows our IDN whitelist to be defeated, which seems pretty bad.  Should this block the FF 1.5 release?

Comment 6

[Darin Fisher]

Attachment: Patch rv1.1

Here's a slightly revised version of the patch.  The main problem with the v1.0 patch was that it was passing |result| as both an in-param as well as an out-param to ConvertUTF8toACE.  That could cause a problem in the future if ConvertUTF8toACE ever modified it's out-param before reading from the in-param, so I fixed the code to use a temporary string object.  I also added a few more comments.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

thanks, Darin.

Comment 8

[Darin Fisher]

Comment on attachment 202136 [details] [diff] [review]
Patch rv1.1

I think this patch is not good.  It introduces cases where .com domain names are displayed as unicode and sent to the DNS resolver as punycode.  See bug 315440.

Comment 9

[Darin Fisher]

Attachment: Patch rv1.2

OK, here's a revised patch.  I am falling back on ConvertUTF8toACE when Normalize fails.  This seems to address the problems that I found with malformed bidi text in bug 315440.

Masayuki: Can you please verify that this patch works as you expect.  Thanks.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

This patch works fine for this patch.
But http://www.%E3%83%8F%E3%83%B3%E3%83%89%E3%83%9C%E3%83%BC%E3%83%AB%E3%82%B5%E3%83%A0%E3%82%BA.com
 is displaied UTF-8 URL on statusbar.

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

> This patch works fine for this patch.

for this *bug*.

Comment 12

[Scott MacGregor]

Is the seriousness of this bug mitigated somewhat by the fact that this is only a problem with urls the user enters by hand (as opposed to URLS the user clicks on from a web page and URLs that come from other applications (like mail)?

Comment 13

[Jungshik Shin]

Comment on attachment 202187 [details] [diff] [review]
Patch rv1.2

r=jshin

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #12)
> Is the seriousness of this bug mitigated somewhat by the fact that this is only
> a problem with urls the user enters by hand (as opposed to URLS the user clicks
> on from a web page and URLs that come from other applications (like mail)?
> 

On thunderbird, if a text mail has IDN URL in its content, thunderbird makes link the URL. But the URL cannot be opened on Firefox by clicking the URL. Maybe, it's another bug.
# I think that the bug is same the bug of comment 10.

Comment 15

[Darin Fisher]

> # I think that the bug is same the bug of comment 10.

Yeah, the URL with %-escaped characters in the hostname cannot be loaded because Mozilla does not unescape the hostname portion of a URL.

Comment 16

[Scott MacGregor]

my question wasn't about thunderbird at all. I was trying to ask if the bypassing of our IDN whitelist check is only happening for URLs a user manually types into the urlbar vs. urls clicked on from a web page or urls we are given by the OS as a result of a user clicking on urls from other applications (like mail, IM, etc).

Comment 17

[Darin Fisher]

(In reply to comment #16)
> my question wasn't about thunderbird at all. I was trying to ask if the
> bypassing of our IDN whitelist check is only happening for URLs a user manually
> types into the urlbar vs. urls clicked on from a web page or urls we are given
> by the OS as a result of a user clicking on urls from other applications (like
> mail, IM, etc).

mscott:  I think that it is possible for any website to give the user links that contain un-normalized unicode characters in the hostname.  That means that, yes, our whitelist is *completely defeated* by this bug.  So, I think this is a serious bug that needs to block the 1.5 release.

Comment 18

[Darin Fisher]

Masayuki: Can you please confirm that this bug could be triggered by web content as well?

Comment 19

[u115577]

Attachment: testcase

This bug can be reproduced by the links on web page.

Comment 20

[Scott MacGregor]

The subject implies that this bug effects urls typed into the url bar by the user only. We should clarify the summary if that's not the case Masayuki. 

Comment 21

[u115577]

Attachment: testcase 2

another tests added.

Comment 22

[Darin Fisher]

OK, I confused myself.  Comment #17 is wrong.  Without this patch, the only harm is that we will show punycode when we could safely show unicode to the user.  Therefore, this is really just a minor bug that websites can workaround.  I don't think we need to block FF 1.5 for this.  Sorry for the false alarm!

Comment 23

[Darin Fisher]

fixed-on-trunk

Comment 24

[u115577]

This bug was reported first by .jp registry to Mozilla Japan. IDN whitelist is a much-anticipated new feature and they hope the problem is fixed in Firefox 1.5, of course. Mozilla Japan also request blocking.

Comment 25

[Scott MacGregor]

because this is a case of us being over zealous as opposed to the white list being bypassed to show things in unicode that should be show in punycode, we aren't going to respin and slip the release for this fix. Let's look into getting it into the next follow up release.

Comment 26

[u115577]

ugh. we'll escalate the issue to MoCo through official channels...

Comment 27

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Let's go to next minor release. This bug was reported by JPRS.

Comment 28

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I filed a bug of comment 10. See bug 315666.

Comment 29

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #20)
> The subject implies that this bug effects urls typed into the url bar by the
> user only. We should clarify the summary if that's not the case Masayuki. 
> 

Oops. Sorry. The old summary is wrong. This is reproduced by the link of web contents too. But this is not a security bug.

I think that we should go to 1.8.1, this bug and bug 315666.

Comment 30

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 202187 [details] [diff] [review]
Patch rv1.2

This is minor problem. But the patch doesn't have regression report. And this bug is reported by JPRS.

Comment 31

[Daniel Veditz [:dveditz]]

Comment on attachment 202187 [details] [diff] [review]
Patch rv1.2

Not for 1.8.0.1 (candidates in hand).

Comment 32

[Daniel Veditz [:dveditz]]

Comment on attachment 202187 [details] [diff] [review]
Patch rv1.2

approving for 1.8.0 branch, a=dveditz for drivers

Comment 33

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

-> v.(trunk and 1.8 branch)

Comment 34

[Jay Patel [:jay]]

v.fixed on 1.8.0 branch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.2) Gecko/20060308 Firefox/1.5.0.2, links in testcase 2 work as expected.