Closed
Bug 369028
Opened 18 years ago
Closed 18 years ago
crash in [@ nsImapUrl::ParseNumBytes]
Categories
(MailNews Core :: Networking: IMAP, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: Bienvenu, Assigned: Bienvenu)
Details
(Keywords: crash, fixed1.8.1.2, verified1.8.1.3, Whiteboard: [sg:nse dos][needs testcase])
Crash Data
Attachments
(1 file)
793 bytes,
patch
|
mscott
:
superreview+
|
Details | Diff | Splinter Review |
embedding a certain imap url string in a mail message can lead to a null ptr deref crash when we try to parse the url, just from displaying the message. We're not trying to run the url, just creating it. Patch upcoming.
Attachment #253689 -
Flags: superreview?(mscott)
Updated•18 years ago
|
Attachment #253689 -
Flags: superreview?(mscott) → superreview+
Assignee | ||
Updated•18 years ago
|
Assignee | ||
Updated•18 years ago
|
Attachment #253689 -
Flags: approval1.8.0.11?
Comment 1•18 years ago
|
||
Why the security flag? This appears to be a straightforward null deref unless I'm missing something.
Whiteboard: [sg:nse]
Assignee | ||
Comment 2•18 years ago
|
||
It's a DOS - if that doesn't warrant the security flag, then I clear it...
Comment 3•18 years ago
|
||
we don't need to hide crashers that can be easily cleared (by deleting the message with the content view). If it's something that can get stuck in there and crash the user until they hand-edit their mailbox or something equally persistent then that would be worth hiding until fixed.
Comment 4•18 years ago
|
||
Comment on attachment 253689 [details] [diff] [review]
proposed fix
approved for the 1.8.0 branch, a=dveditz for drivers.
We have not built TBird 1.5.0.10 yet, can you land this in the next couple of hours?
Attachment #253689 -
Flags: approval1.8.0.11? → approval1.8.0.10+
Assignee | ||
Comment 5•18 years ago
|
||
Comment on attachment 253689 [details] [diff] [review]
proposed fix
clearing approval - my bad, this doesn't apply to the 1.5.0.x branch
Attachment #253689 -
Flags: approval1.8.0.10+
Comment 6•18 years ago
|
||
David: Any easy way to test this? So we have a testcase that crashes? If not, the patch looks trivial, so we can just mark this verified by code inspection. Let me know.
Whiteboard: [sg:nse dos] → [sg:nse dos][needs testcase]
Assignee | ||
Comment 7•18 years ago
|
||
sorry, this turns out not to be an issue at all for 1.5.0.x - for 2.0, it's definitely fixed - as you say, the patch is trivial. The test case I have has private data in it, but basically, it was an imap log that had imap urls in it to fetch preview text, and creating those urls when the message was displayed was causing the crash.
Comment 8•18 years ago
|
||
David: Since you have the test case in question, could you run it against the candidate build for a final verification?
(In reply to comment #7)
> sorry, this turns out not to be an issue at all for 1.5.0.x - for 2.0, it's
> definitely fixed - as you say, the patch is trivial. The test case I have has
> private data in it, but basically, it was an imap log that had imap urls in it
> to fetch preview text, and creating those urls when the message was displayed
> was causing the crash.
>
Summary: crash in nsImapUrl::ParseNumBytes() → crash in [@ nsImapUrl::ParseNumBytes]
Comment 10•18 years ago
|
||
Per Comment 9, adding the branch verified keyword.
Keywords: verified1.8.1.3
Updated•16 years ago
|
Product: Core → MailNews Core
Updated•14 years ago
|
Crash Signature: [@ nsImapUrl::ParseNumBytes]
You need to log in
before you can comment on or make changes to this bug.
Description
•