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

RESOLVED FIXED in mozilla1.9.1a2

Status

()

Core
Internationalization
--
minor
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Christoph Garst, Assigned: smontagu)

Tracking

({fixed1.8.1.17, fixed1.9.0.2, regression})

unspecified
mozilla1.9.1a2
x86
Linux
fixed1.8.1.17, fixed1.9.0.2, regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

9 years ago
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?= <foo@example.com>
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).
(Assignee)

Comment 1

9 years ago
Created attachment 333167 [details] [diff] [review]
Patch
Attachment #333167 - Flags: review?(VYV03354)
(Assignee)

Updated

9 years ago
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Reporter)

Comment 2

9 years ago
Works, thanks.
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?
(Assignee)

Comment 4

9 years ago
Created attachment 333250 [details] [diff] [review]
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?
Attachment #333167 - Attachment is obsolete: true
Attachment #333250 - Flags: review?(VYV03354)
Attachment #333167 - Flags: review?(VYV03354)
(Reporter)

Comment 5

9 years ago
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 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.
Attachment #333250 - Flags: review?(VYV03354) → review+
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
Pushed as 17110:b17a3d22447e.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1a2
(Assignee)

Comment 8

9 years ago
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.
Attachment #333250 - Flags: approval1.9.0.2?
Attachment #333250 - Flags: approval1.8.1.17?
Keywords: regression
Blocks: 381412
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.
Attachment #333250 - Flags: approval1.9.0.2?
Attachment #333250 - Flags: approval1.9.0.2+
Attachment #333250 - Flags: approval1.8.1.17?
Attachment #333250 - Flags: approval1.8.1.17+
(Assignee)

Updated

9 years ago
Keywords: fixed1.8.1.7, fixed1.9.0.2
Keywords: fixed1.8.1.7 → fixed1.8.1.17
You need to log in before you can comment on or make changes to this bug.