Closed Bug 387403 Opened 14 years ago Closed 14 years ago
Thunderbird eat memory until crash when opening e-mail with broken vcard
1.02 KB, text/plain
532 bytes, patch
|Details | Diff | Splinter Review|
1.68 KB, patch
|Details | Diff | Splinter Review|
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:220.127.116.11) Gecko/20070603 Fedora/18.104.22.168-2.fc7 Firefox/22.214.171.124 Build Identifier: 126.96.36.199 (20070604) When opening e-mail with broken vcard that looks like this: ========================================================================== begin:vcard fn;quoted-printable:Xxxx=C5=82xx Xxx n;quoted-printable:Xxx;Xxxx=C5=82xx adr;quoted-printable;quoted-printable;dom:;;xx. Xxxxxxxxxxxx X;Lubacz=C3=3 ========================================================================== Thunderbird eats all available memory. This is an actual vcard received from another Thunderbird 188.8.131.52 user (personal details changed to X-es). I don't know how this user has managed to create a vcard that broken (a city name ends with a half of a letter "ó" (oacute), it should be "Lubaczów"), but it seems it is somehow possible. Thunderbird should not crash on an e-mail anyway - does not matter how terribly broken one. Reproducible: Always Steps to Reproduce: 1. Open the message I'll attach. Actual Results: Thunderbird eats all available memory, slows a computer to a crawl and then crashes. Expected Results: Message is displayed, possibly with an information that attached vcard is broken. I've tested Thunderbird 184.108.40.206 on Windows and Linux and Seamonkey 1.1.2 on Windows - all affected. I think this could be used as some kind of DOS.
yeah, this is a simple DOS - marking security sensitive for now, though the work around is not to click on the message. The bug looks to be in mailnews/addrbook/src/nsvcard.cpp
Status: UNCONFIRMED → NEW
Ever confirmed: true
this fixes the infinite allocating loop. We won't display any of the v-card because basically the v-card parser seems to ignore the whole thing in case of errors. I'm not sure if this is going to break a v-card where we have an empty field at the end, but I don't think that would be valid anyway, since the vcard should end with "end:vcard".
Assignee: nobody → bienvenu
Status: NEW → ASSIGNED
Attachment #271530 - Flags: superreview?(mscott)
Attachment #271530 - Flags: superreview?(mscott) → superreview+
fixed on trunk - requesting 220.127.116.11 approval, after some bake time on the trunk.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Does this affect Thunderbird 18.104.22.168 as well?
good question, yes, from looking at the code, I think it does (i.e., it's not a regression)
Comment on attachment 271530 [details] [diff] [review] possible fix approved for 22.214.171.124 and 126.96.36.199, a=dveditz for release-drivers. Please land within ~30 hours or it'll have to go next time.
fix landed on 188.8.131.52 branch
verified fixed 184.108.40.206 using the test email message from comment #1 with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:220.127.116.11pre) Gecko/20070711 Thunderbird/18.104.22.168pre ID:2007071104 on Fedora F7 and Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:22.214.171.124pre) Gecko/20070710 Thunderbird/126.96.36.199pre Mnenhy/0.7.5.0 ID:2007071100 - Thunderbird don`t crash or eat all memory. Thunderbird stays here with 44 MB Memory Usage. -> adding verified keyword
This still needs landing on the 1.8.0 branch for an eventual Thunderbird 188.8.131.52
Checked in to 1.8.0 branch
verified fixed Mozilla/5.0 (Windows; U; Windows NT 5.1; zh-CN; rv:184.108.40.206) Gecko/20070809 Thunderbird/220.127.116.11 ID:2007080918 Thunderbird does not crash and also not eat all memory
This FIXED bug is flagged with in‑testsuite? It would be great if assignee or someone else can clear the flag if a test is not appropriate. And if appropriate, create a test and plus the flag to finish off the bug.
Testcase for this bug. With the "possible fix" patch backed out, this test hangs and eats memory. Note that I've downgraded an assertion in nsAbManager to a warning - if the vcard parsing failed, then it is potentially not us (a lot of vcards come from outside). I didn't want to change the return result as looking at the callers they don't handle an error situation, so just picking up a blank card is best (and I doubt we'd have got this far anyway because we'd be attempting to parse a vcard displayed on a message).
Sorry, here's the correct version.
Comment on attachment 367858 [details] [diff] [review] [checked in] Testcase v2 Checked in testcase: http://hg.mozilla.org/comm-central/rev/5cb4e78cb271
Attachment #367858 - Attachment description: Testcase v2 → [checked in] Testcase v2
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.