Crash, possible buffer overrun in comi18n.cpp

VERIFIED FIXED in mozilla1.4beta

Status

()

Core
Security
VERIFIED FIXED
14 years ago
13 years ago

People

(Reporter: Mitchell Stoltz (not reading bugmail), Assigned: Cavin Song)

Tracking

Trunk
mozilla1.4beta
x86
Windows 2000
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

1.51 KB, text/plain
Details
4.68 KB, text/plain
Details
746 bytes, patch
Cavin Song
: review+
jag (Peter Annema)
: superreview+
(not reading, please use seth@sspitzer.org instead)
: approval1.4b+
Details | Diff | Splinter Review
Attached file tmp3.pl generates email message which crashes mozilla.
Depending on the length of the overflow, some time mozilla exits, sometimes
it is possible to attach with gdb.
Not sure where the overflow is.


Found this while examinig comi18n.cpp, though the crash may not be related
to it.
In comi18n.cpp in function intl_decode_mime_part2_str there is
--------------------
 /* Assume no more than 3X expansion due to UTF-8 conversion */
 retbuff = (char *)PR_Malloc(3*strlen(header)+1);
--------------------
Then in intl_copy_uncoded_header the string is converted to UTF-8 and
copied.

Is such an attack possible:
Embed \x00 in the headers so strlen terminates and then \x00 is treated as
valid UTF-8 or some other charset character ?
Looks like it is possible to embed \x00 in From: and other headers - it
does not terminate the string when the message is opened.
Not sure whether the comi18n is hit, though.


Georgi Guninski
(Reporter)

Comment 1

14 years ago
Created attachment 120978 [details]
Testcase
Seth, another possible buffer overrun in MailNews. Can you investigate, or get
someone else from the mailnews people to check this out?
(Reporter)

Updated

14 years ago
Flags: blocking1.4b?
re-assign to cavin, to investigate.
Assignee: mstoltz → cavin
Keywords: nsbeta1
Target Milestone: --- → mozilla1.4beta

Comment 4

14 years ago
4 bytes per Unicode character is a maximum for UTF-8 encoding, so it is safer to
do  times four instead of three
but existing charset encoding's code point maps to maximum 3 bytes UTF-8 (so no
4 byte case from one character in the input charset code point)
Simon, any comment?
Created attachment 121579 [details]
Stack trace
(Assignee)

Comment 6

14 years ago
Ah ok, this is where it crashes. Thanks Simon. I could not see the crash on my
machine with debug build. Using the test mail msg the mime/i18n code seems to
work fine (ie, buffer is big enough) because ConvertToUnicode() seems to fail
all the time so the data actually get copies to the buffer is very small.

Naoki's comment #4 indicates that *3 is pretty safe.
Created attachment 121587 [details] [diff] [review]
Patch

Updated

14 years ago
Attachment #121587 - Flags: superreview?(jaggernaut)
Attachment #121587 - Flags: review?(cavin)
(Assignee)

Comment 8

14 years ago
Comment on attachment 121587 [details] [diff] [review]
Patch

r=cavin. Just curious, what's 'currentCharset' set to when it crashes (since I
can't see it on my machine)?
(Assignee)

Comment 9

14 years ago
Comment on attachment 121587 [details] [diff] [review]
Patch

r=cavin. Just curious, what's 'currentCharset' set to when it crashes (since I
can't see it on my machine)?
Attachment #121587 - Flags: review?(cavin) → review+
Without the patch, currentCharset is pointing at uninitialized memory when it
crashes.
Adding jag to cc in case that was blocking the review request.

Comment 12

14 years ago
Comment on attachment 121587 [details] [diff] [review]
Patch

sr=jag
Attachment #121587 - Flags: superreview?(jaggernaut) → superreview+
Comment on attachment 121587 [details] [diff] [review]
Patch

Requesting 1.4 approval for this simple crash fix.
Attachment #121587 - Flags: approval1.4b?
Comment on attachment 121587 [details] [diff] [review]
Patch

a=sspitzer
Attachment #121587 - Flags: approval1.4b? → approval1.4b+

Comment 15

14 years ago
Can someone please land this on the 1.3.1 branch as well (ASAP)? Thanks.
http://bugzilla.mozilla.org/attachment.cgi?id=121587&action=view
fix landed on 1.3 branch
Fix checked in to trunk.
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED

Updated

14 years ago
Flags: blocking1.4b?

Comment 18

14 years ago
verified
Status: RESOLVED → VERIFIED
Bugs published on the Known-vulnerabilities page long ago, removing confidential
flag.
Group: security
You need to log in before you can comment on or make changes to this bug.