1404349 - "moderately restrictive" IDN spoofing with U+05B4 (Hebrew NSM) despite fix for bug 1370497

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect)

Description

[Khalil Zhani]

Attachment: Screen Shot 2017-09-29 at 15.56.03.png

Firefox should not allows mixing latin with hebrew character as last fix in bug 1370497.

http://www.gooִgle.com (U+05B4)

Comment 1

[Daniel Veditz [:dveditz]]

How is this not a duplicate of bug 1370497? In nightly I get Punycode.

Comment 2

[Khalil Zhani]

(In reply to Daniel Veditz [:dveditz] from comment #1)
> How is this not a duplicate of bug 1370497? In nightly I get Punycode.

I was able to repro it before fix bug 1399939, also, on Chrome they disallowed Arabic/Hebrew NSMs not only Arabic https://bugs.chromium.org/p/chromium/issues/detail?id=729979#c18.

Comment 3

[Daniel Veditz [:dveditz]]

STR: set the pref network.IDN.restriction_profile to "moderate" (the default before bug 1399939 changed it to "high") and open the link from comment 0: http://www.gooִgle.com

(In reply to Khalil Zhani from comment #2)
> on Chrome they disallowed Arabic/Hebrew NSMs not only Arabic

The fix in bug 1370497 was not specific to Arabic, although the testcase was.

Jonathan: why is U+05B4 slipping through that code? They appear to be marked similarly in the Unicode tables:

05B4;HEBREW POINT HIRIQ;Mn;14;NSM;;;;;N;;;;;
0650;ARABIC KASRA;Mn;32;NSM;;;;;N;ARABIC KASRAH;;;;

It's a little bit of a moot point since bug 1399939 has landed and also seems to prevent this, but I worry we're missing something here.

Comment 4

[Jonathan Kew [:jfkthame]]

There's a bug in the patch that landed in bug 1370497: it works as intended for diacritics that have Script=INHERITED as their primary Unicode script property (which includes the Arabic vowels that are also used in Syriac, for example); but fails for diacritics that have a specific Script property (such as this Hebrew example), because in that case, the `lastScript` variable has already been updated to the Script value of the diacritic before the new check is performed.

It's easy to fix this, by deferring the update of the `lastScript` variable until the end of the loop, after the diacritic-script check has been done (if applicable).

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: Extend check from bug 1370497 to apply to marks with a specific Script property, as well as those with Script=Inherited

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e92d092fdacc

Comment 7

[Daniel Veditz [:dveditz]]

Although a valid bug in our earlier fix, it was also effectively "fixed" by bug 1399939 and unfortunately does warrant a bounty.

Comment 8

[Daniel Veditz [:dveditz]]

Although a valid bug in our earlier fix, it was also effectively "fixed" by bug 1399939 and unfortunately does not warrant a bounty.

Comment 9

[Bogdan Maris, Desktop QA]

I reproduced this issue on Nightly 58.0a1 (2017-09-29) under macOS 10.13, using steps from comment 3. The issue is fixed on Release 58.0, Beta 58.0b16 and on latest Nightly (2018-01-22) under macOS 10.13, macOS 10.12 and OS X 10.11.