Mail Import Crashes with Divide by Zero

RESOLVED FIXED

Status

--
critical
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: dh4ca, Assigned: gwenger)

Tracking

x86
Windows NT

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-penelope)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

12 years ago
User-Agent:       Opera/9.10 (Windows NT 4.0; U; en)
Build Identifier: 2.0.0.0 (20070326)

With the sample Eudora inbox, the mail import function crashes with a Divide by Zero error.  Dr. Watson for Windows NT reports it as "Exception: divide by zero (0xc0000094), Address: 0x00976479".  The problem is caused by a malformed Content-Type header which has been merged with a Message-ID header on a single line.  Adding a Newline in front of the Message-Id header resolves the problem.

Reproducible: Always

Steps to Reproduce:
1.Import the sample Eudora inbox.
2.
3.



I can't tell if the malformed headers were mangled by Eudora or by the originating mail client/server but I eventually found a total of five messages in my Eudora mailboxes which were all mangled in the same way.  Although the workaround is relatively easy, it was a painful introduction to Thunderbird and not one that you should inflict on many more Eudora refugees (or other newbies if it's not a Eudora specific issue).
(Reporter)

Comment 1

12 years ago
Created attachment 264473 [details]
Sample Eudora inbox for this bug
(Reporter)

Updated

12 years ago
Version: unspecified → 2.0

Comment 2

12 years ago
Not sure if this is fixed or not.
(Assignee)

Comment 3

12 years ago
Reassigned bug to me. I'll take a look at it. In general I would say feel free to assign any Eudora importer bugs to me.
Assignee: mscott → gwenger
(Assignee)

Updated

11 years ago
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Assignee)

Comment 4

11 years ago
Created attachment 274822 [details] [diff] [review]
[checked in] Patch that fixes bug. Note patch was not created with CVS.

Added maximum length sanity check for charset parameter of Content-Type header when importing.
Attachment #274822 - Flags: superreview?
Attachment #274822 - Flags: review?
(Assignee)

Comment 5

11 years ago
Comment on attachment 274822 [details] [diff] [review]
[checked in] Patch that fixes bug. Note patch was not created with CVS.

Added maximum length sanity check for charset parameter of Content-Type header when importing.
Attachment #274822 - Flags: superreview?(bienvenu)
Attachment #274822 - Flags: superreview?
Attachment #274822 - Flags: review?(bienvenu)
Attachment #274822 - Flags: review?
(Assignee)

Updated

11 years ago
Whiteboard: fixed-penelope

Comment 6

11 years ago
Geoffrey, where are we crashing? Do you have a stack trace? I'm worried that we're papering over a problem elsewhere in the code - the related code in the diff shouldn't care how long the content type is, so the crash must be elsewhere...
(Assignee)

Comment 7

11 years ago
It was crashing in "apply_rfc2047_encoding", which is in comi18n.cpp. The problem was a divide by zero in this line:
PRInt32 maxNumLines = (strlen(src) / charsPerLine) + 1;

charsPerLine was zero when perLineOverhead was just the right (or wrong) value (I think 72).

On the importers end, the long charset value was clearly bad data, so I thought it was justified to fix it there. I do, however, think that apply_rfc2047_encoding should be made more robust by checking and avoiding that possibility too.

Comment 8

11 years ago
Comment on attachment 274822 [details] [diff] [review]
[checked in] Patch that fixes bug. Note patch was not created with CVS.

this makes sense; I'll land it on the trunk, thx. I'll probably remove the part of the comment that refers to a bug # - we don't do that much, since cvsblame usually suffices. I'll also attach a fix that prevents the crash.
Attachment #274822 - Flags: superreview?(bienvenu)
Attachment #274822 - Flags: superreview+
Attachment #274822 - Flags: review?(bienvenu)
Attachment #274822 - Flags: review+

Comment 9

11 years ago
Created attachment 275462 [details] [diff] [review]
fix divide by 0 error
Attachment #275462 - Flags: superreview?(mscott)

Updated

11 years ago
Attachment #274822 - Attachment description: Patch that fixes bug. Note patch was not created with CVS. → [checked in] Patch that fixes bug. Note patch was not created with CVS.

Updated

11 years ago
Attachment #275462 - Flags: superreview?(mscott) → superreview+

Comment 10

11 years ago
Created attachment 281237 [details] [diff] [review]
patch as checked in.

I just noticed I attached the wrong diff before - this is what was checked in.
Attachment #275462 - Attachment is obsolete: true

Comment 11

11 years ago
marking fixed
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.