449578 - Wrong interpretation of escape sequence in charset iso-2022-kr

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Christoph Garst]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.14) Gecko/20061201 Firefox/2.0.0.14 (Ubuntu-feisty)
Build Identifier: Thunderbird 2.0.0.16 (20080724)

Thunderbird displays an unicode replacement character in front of each e-mail-header which has charset iso-2022-kr.

Reproducible: Always

Steps to Reproduce:
1. Compose an email with following To-Header: =?iso-2022-kr?q?=1B=24=29C=0E "some korean characters" =0F?= <[e-mail]>
2. View that mail in Thunderbird

Actual Results:  
There will be an additional unicode replacement character in front of the korean text

Expected Results:  
Don't show that character

In intl/uconv/ucvko/nsISO2022KRToUnicode.cpp at lines 107 and 132 there is a check if mRunLength == 0 (Length of non-ASCII run). The unicode replacement character is added if the decoder finds the escape sequence (ESC "$" ")" "C") in front of a line (where it should be, as rfc1557 states).

Comment 1

[Simon Montagu :smontagu]

Attachment: Patch

Comment 2

[Christoph Garst]

Works, thanks.

Comment 3

[Masatoshi Kimura [:emk]]

Comment on attachment 333167 [details] [diff] [review]
Patch

How to prevent a XSS attack like bug 381412? Indeed, the sequence ESC "$" ")" "C" can occur any number of times per RFC 1557, I'm hesitating to allow appearing the designator twice (or more).
BTW I'm not a intl owner/peer. Is jshin inactive now?

Comment 4

[Simon Montagu :smontagu]

Attachment: Improved patch

What do you think of this approach, only allowing ESC $)C at the beginning of a line? I am a little hesitant because I don't have any examples of actual ISO-2022-KR mails to test against, and I don't know how well existing clients that send ISO-2022-KR mails conform to RFC 1557. Christoph, maybe you can attach some samples or forward them to me?

Comment 5

[Christoph Garst]

This would comply to RFC 1557 and works for me. I use gmime to create mails which uses iconv to convert from the internal utf-8 representation to a best fitting charset (my characters are japanese, but gmime encodes to iso-2022-kr). I don't have another sample and i don't know how wide spread this charset really is (this bug would have been found before).

Comment 6

[Masatoshi Kimura [:emk]]

Comment on attachment 333250 [details] [diff] [review]
Improved patch

Sorry for the late response.
I think it's a reasonable compromise. ISO-2022-KR is rarely used in the real world.

Comment 7

[Reed Loden [:reed]]

Pushed as 17110:b17a3d22447e.

Comment 8

[Simon Montagu :smontagu]

Comment on attachment 333250 [details] [diff] [review]
Improved patch

This is a regression from bug 381412, so we need the fix on the branches where that went in.
The testcase includes a test for the regression and for issues similar to those in bug 381412.

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 333250 [details] [diff] [review]
Improved patch

Approved for 1.8.1.17 and 1.9.0.2, a=dveditz for release-drivers.