Closed
Bug 257314
Opened 20 years ago
Closed 20 years ago
stack based buffer overflow with vcards when previewing email message
Categories
(Core :: Security, defect)
Core
Security
Tracking
()
VERIFIED
FIXED
People
(Reporter: guninski, Assigned: dveditz)
References
Details
(Keywords: fixed-aviary1.0, fixed1.7.3, Whiteboard: [sg:fix])
Attachments
(2 files)
1.03 KB,
text/x-vcard
|
Details | |
955 bytes,
patch
|
mscott
:
review+
dveditz
:
superreview+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040115 Build Identifier: Mozilla/5.0 There is stack based bufferoverflow in mozilla mail when handling vcards. The stack is quite screwed. It is triggered by just previewing a specially crafted message. Tested on mozilla 1.7 (because 1.7.2 fails to build due to missing directories), but the code seems present in 1.7.2 and thunderbird 0.7 also. Obviously the following code is quite fucked: -------- addrbook/src/nsVCardObj.cpp static void writeGroup(OFile *fp, VObject *o) { char buf1[256]; char buf2[256]; PL_strcpy(buf1,NAME_OF(o)); while ((o=isAPropertyOf(o,VCGroupingProp)) != 0) { PL_strcpy(buf2,STRINGZ_VALUE_OF(o)); PL_strcat(buf2,"."); PL_strcat(buf2,buf1); PL_strcpy(buf1,buf2); } appendsOFile(fp,buf1); } -------- To reproduce send the vcard fuckmicrosft.vcf (which will be shortly attached) as attachment and then preview the message. -------------------------------------------------- (gdb) info stack #9 0x45962c5b in initPropIterator (i=0xbfffe2a8, o=0x67676767) at nsVCardObj.cpp:383 #10 0x45962d2a in isAPropertyOf (o=0x67676767, id=0x4598bc69 "grouping") at nsVCardObj.cpp:418 #11 0x45964552 in writeGroup (fp=0x67676767, o=0x67676767) at nsVCardObj.cpp:1350 #12 0x67676767 in ?? () #13 0x67676767 in ?? () #14 0x67676767 in ?? () #15 0x67676767 in ?? () #16 0x67676767 in ?? () #17 0x67676767 in ?? () #18 0x67676767 in ?? () ------------------------------------------------- georgi Reproducible: Always Steps to Reproduce: 1. will attach a testcase Actual Results: stack is screwed Expected Results: stack should not be screwed
Reporter | ||
Comment 1•20 years ago
|
||
vcard.vcf which causes stack overflow when previewed in mail message.
Comment 2•20 years ago
|
||
I have the preview pane turned on and had no problems with the message George sent to the security list. Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040616 Gerv
Reporter | ||
Comment 3•20 years ago
|
||
Gerv: are you showing attachments inline? The default option is to show attachments inline i believe.
Comment 4•20 years ago
|
||
Heh, the email to security@mozilla kept crashing me every time I tried to open it. I finally read the summary and checked the bug before trying to read the mail again. I can reproduce a crash viewing Georgi's titled "bug 257314" on a Thunderbird branch build from yesterday.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reporter | ||
Comment 5•20 years ago
|
||
the same fucked code is in: other-licenses/libical/src/libicalvcal/vobject.c http://lxr.mozilla.org/seamonkey/source/other-licenses/libical/src/libicalvcal/vobject.c#1253 probably it also should be fixed.
Comment 6•20 years ago
|
||
This is a patch to use string class instead of char arrays for addrbook/src/nsVCardObj.cpp.
Comment 7•20 years ago
|
||
I checked the original libical package libical-0.24.RC4.tar.gz and it has the same bad code as the one in other-licenses. Libical developers should be notified about that, right?
Updated•20 years ago
|
Attachment #157352 -
Flags: review?(dveditz)
Comment 8•20 years ago
|
||
Looks to me like other-licenses/libical/src/libicalvcal/vobject.c is not used at least in SeaMonkey with calendar.
Comment 9•20 years ago
|
||
Simple HTML not vulnerable.
Flags: blocking1.8a4?
Flags: blocking1.7.x?
Flags: blocking-aviary1.0mac?
Flags: blocking-aviary1.0PR?
Flags: blocking-aviary1.0?
Assignee | ||
Comment 10•20 years ago
|
||
Comment on attachment 157352 [details] [diff] [review] Patch for addrbook/src/nsVCardObj.cpp I'm not a mailnews peer, let's get the r= from mscott;I'll sr= scott: while you're reviewing, please check out nsDirPrefs.cpp in the same directory which also has a lot of unchecked strcats and strcpys. Less susceptible to a remote hacker, but is there any way these prefs could exceed 128 chars? Seems possible since some are hostname based and I've seen some exceptionally long hostnames (though rare). The longest ldap pref I currently have is 60 chars so it's unlikely to overflow by accident, but hackers are not accidental. ("Get your free LDAP here! e-mail addresses of your favorite celebrities!"). Anyway, back to this... >+ while ((o=isAPropertyOf(o,VCGroupingProp)) != 0) { >+ buf.Insert(NS_LITERAL_CSTRING("."), 0); >+ buf.Insert(STRINGZ_VALUE_OF(o), 0); What are the guarantees about this data structure? Since Insert is slow if we're sure o->val.strs is non-null you could combine these using nsDependentString but.Insert(nsDependentString(STRINGZ_VALUE_OF(o) + NS_LITERAL_CSTRING("."), 0); But what you have works and isn't going to be called much, so unless we're sure I'm happy with the patch as-is sr=dveditz
Attachment #157352 -
Flags: superreview+
Attachment #157352 -
Flags: review?(mscott)
Attachment #157352 -
Flags: review?(dveditz)
Assignee | ||
Comment 11•20 years ago
|
||
I can at least say it's blocking 1.7x and 1.8a4; I'm going out on a limb and making this block ff 1.0PR and 1.7.3 as well and let Asa/BenG clear it.
Blocks: sbb?
Flags: blocking1.8a4?
Flags: blocking1.8a4+
Flags: blocking1.7.x?
Flags: blocking1.7.x+
Flags: blocking-aviary1.0PR?
Flags: blocking-aviary1.0PR+
Whiteboard: [sg:fix]
Comment 12•20 years ago
|
||
Comment on attachment 157352 [details] [diff] [review] Patch for addrbook/src/nsVCardObj.cpp I'm not positive that the old vcard string code can promise that these strings are null terminated, so like Dan I'm afraid to use the nsDependentString shortcut. To answer Dan's other question, those other calls could be problematic too. Maybe we should get another bug going for someone to step through the debugger and take a closer look.
Attachment #157352 -
Flags: review?(mscott) → review+
Comment 13•20 years ago
|
||
Patch checked in to SeaMonkey trunk. Could someone drive it in for the others?
Assignee | ||
Comment 15•20 years ago
|
||
Checked in on 1.7 and 1.7.2 branches
Keywords: fixed-aviary1.0,
fixed1.7.x
Whiteboard: [sg:fix],fixed-aviary1.0 → [sg:fix] fixed1.7.2+
Assignee | ||
Comment 16•20 years ago
|
||
fixed on trunk per comment 13
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Updated•20 years ago
|
Flags: blocking-aviary1.0mac?
Flags: blocking-aviary1.0?
Assignee | ||
Updated•20 years ago
|
Group: security
Whiteboard: [sg:fix] fixed1.7.2+ → [sg:fix] fixed1.7.3
Comment 17•20 years ago
|
||
re: comment 5 and comment 7 about fixing libical upstream it looks like the bad code that georgi uncovered was already found and fixed by KDE long ago. unfortunately, it didn't make it to the libical CVS until shortley after their last release :( according to http://cvs.sourceforge.net/viewcvs.py/freeassociation/libical/src/libicalvcal/vobject.c rev 1.4 of freeassociation/libical/src/libicalvcal/vobject.c was checked in on 12/16/02 with the following comment: -- Fix possible buffer overflows. This comes from a code review of KDE. Obtained from: Waldo Bastian <bastian@kde.org> -- diff available at http://cvs.sourceforge.net/viewcvs.py/freeassociation/libical/src/libicalvcal/vobject.c?r1=1.3&r2=1.4 should waldo bastian also be given credit at http://www.mozilla.org/projects/security/known-vulnerabilities.html ? btw: i opened Bug 263515 to track updating of other-licenses/libical
Comment 18•20 years ago
|
||
bug 272691 tracks crediting waldo
Comment 19•20 years ago
|
||
Tested using Mac 1.75 - 20041216 - 09 - looks good to me, no crash when attaching vcard to a mail message and then previewing it.
Assignee | ||
Updated•20 years ago
|
Keywords: fixed1.7.5 → fixed1.7.3
Whiteboard: [sg:fix] fixed1.7.3 → [sg:fix]
You need to log in
before you can comment on or make changes to this bug.
Description
•