Last Comment Bug 202546 - Crash, possible buffer overrun in comi18n.cpp
: Crash, possible buffer overrun in comi18n.cpp
Status: VERIFIED FIXED
:
Product: Core
Classification: Components
Component: Security (show other bugs)
: Trunk
: x86 Windows 2000
: -- normal (vote)
: mozilla1.4beta
Assigned To: Cavin Song
: Charles Rosendahl
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2003-04-18 09:38 PDT by Mitchell Stoltz (not reading bugmail)
Modified: 2004-07-20 04:49 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Testcase (1.51 KB, text/plain)
2003-04-18 09:39 PDT, Mitchell Stoltz (not reading bugmail)
no flags Details
Stack trace (4.68 KB, text/plain)
2003-04-24 14:04 PDT, Simon Montagu :smontagu
no flags Details
Patch (746 bytes, patch)
2003-04-24 15:42 PDT, Simon Montagu :smontagu
cavin: review+
jag-mozilla: superreview+
sspitzer: approval1.4b+
Details | Diff | Review

Description Mitchell Stoltz (not reading bugmail) 2003-04-18 09:38:24 PDT
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
Comment 1 Mitchell Stoltz (not reading bugmail) 2003-04-18 09:39:24 PDT
Created attachment 120978 [details]
Testcase
Comment 2 Heikki Toivonen (remove -bugzilla when emailing directly) 2003-04-18 13:35:03 PDT
Seth, another possible buffer overrun in MailNews. Can you investigate, or get
someone else from the mailnews people to check this out?
Comment 3 (not reading, please use seth@sspitzer.org instead) 2003-04-23 18:09:50 PDT
re-assign to cavin, to investigate.
Comment 4 nhottanscp 2003-04-24 14:04:10 PDT
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?
Comment 5 Simon Montagu :smontagu 2003-04-24 14:04:38 PDT
Created attachment 121579 [details]
Stack trace
Comment 6 Cavin Song 2003-04-24 15:05:03 PDT
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.
Comment 7 Simon Montagu :smontagu 2003-04-24 15:42:59 PDT
Created attachment 121587 [details] [diff] [review]
Patch
Comment 8 Cavin Song 2003-04-24 17:08:04 PDT
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)?
Comment 9 Cavin Song 2003-04-24 17:08:10 PDT
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)?
Comment 10 Simon Montagu :smontagu 2003-04-24 18:57:36 PDT
Without the patch, currentCharset is pointing at uninitialized memory when it
crashes.
Comment 11 Simon Montagu :smontagu 2003-04-24 19:00:15 PDT
Adding jag to cc in case that was blocking the review request.
Comment 12 jag (Peter Annema) 2003-04-24 19:02:41 PDT
Comment on attachment 121587 [details] [diff] [review]
Patch

sr=jag
Comment 13 Simon Montagu :smontagu 2003-04-24 19:10:38 PDT
Comment on attachment 121587 [details] [diff] [review]
Patch

Requesting 1.4 approval for this simple crash fix.
Comment 14 (not reading, please use seth@sspitzer.org instead) 2003-04-24 20:54:29 PDT
Comment on attachment 121587 [details] [diff] [review]
Patch

a=sspitzer
Comment 15 Asa Dotzler [:asa] 2003-04-25 10:27:10 PDT
Can someone please land this on the 1.3.1 branch as well (ASAP)? Thanks.
Comment 16 (not reading, please use seth@sspitzer.org instead) 2003-04-25 10:36:05 PDT
http://bugzilla.mozilla.org/attachment.cgi?id=121587&action=view
fix landed on 1.3 branch
Comment 17 Simon Montagu :smontagu 2003-04-25 11:25:35 PDT
Fix checked in to trunk.
Comment 18 Charles Rosendahl 2003-05-16 15:35:51 PDT
verified
Comment 19 Daniel Veditz [:dveditz] 2004-07-20 04:49:42 PDT
Bugs published on the Known-vulnerabilities page long ago, removing confidential
flag.

Note You need to log in before you can comment on or make changes to this bug.