Closed Bug 1553808 (CVE-2019-11705) Opened 5 years ago Closed 5 years ago

Stack buffer overflow in icalrecur.c icalrecur_add_bydayrules


(Calendar :: General, defect)

Lightning 6.2
Not set


(Not tracked)



(Reporter: u621419, Assigned: darktrojan)



(Keywords: csectype-bounds, reporter-external, sec-high)


(2 files)

Attached file stack-overflow-submit.eml β€”

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:66.0) Gecko/20100101 Firefox/66.0

Steps to reproduce:

Open the attached saved message (stack-overflow-submit.eml) in Thunderbird. Receiving this message in my inbox also triggers the bug without further user interaction. The thunderbird process is killed.

Actual results:

A stack buffer overflow happens when loading the attached file. An specially crafted text/calendar mail attachment triggers a stack buffer overflow write in icalrecur_add_bydayrules.

$ thunderbird stack-overflow-submit.eml
[calBackendLoader] Using Thunderbird's builtin libical backend
*** stack smashing detected ***: <unknown> terminated
Aborted (core dumped

==4191==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff695fb968 at pc 0x00000051e8cb bp 0x7fff695fad50 sp 0x7fff695fad48
WRITE of size 2 at 0x7fff695fb968 thread T0
#0 0x51e8ca in icalrecur_add_bydayrules /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalrecur.c:409:22
#1 0x51f5b0 in icalrecurrencetype_from_string /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalrecur.c:481:6
#2 0x535554 in icalvalue_new_from_string_with_error /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalvalue.c:615:11
#3 0x519251 in icalparser_add_line /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:1075:14
#4 0x517e1b in icalparser_parse /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:623:11
#5 0x4fd243 in icalparser_parse_string /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:1236:9
#6 0x4fa975 in LLVMFuzzerTestOneInput (/opt/libfuzzer/thunderbird_libical_fuzzer+0x4fa975)
#7 0x43a681 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/opt/libfuzzer/thunderbird_libical_fuzzer+0x43a681)
#8 0x424327 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/opt/libfuzzer/thunderbird_libical_fuzzer+0x424327)
#9 0x42a4c1 in fuzzer::FuzzerDriver(int*, char***, int ()(unsigned char const, unsigned long)) (/opt/libfuzzer/thunderbird_libical_fuzzer+0x42a4c1)
#10 0x453f62 in main (/opt/libfuzzer/thunderbird_libical_fuzzer+0x453f62)
#11 0x7f85ccd8ab96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
#12 0x41dc49 in _start (/opt/libfuzzer/thunderbird_libical_fuzzer+0x41dc49)

Address 0x7fff695fb968 is located in stack of thread T0 at offset 2824 in frame
#0 0x51ed23 in icalrecurrencetype_from_string /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalrecur.c:420

This frame has 2 object(s):
[32, 2824) 'parser' (line 421) <== Memory access at offset 2824 overflows this variable
[2960, 3008) 'tmp' (line 462)

Expected results:

No stack overflow.

Flags: sec-bounty?
Component: Untriaged → General
Product: Thunderbird → Calendar
Summary: Stack buffer overflow in icalrecur_add_bydayrules → Stack buffer overflow in icalrecur.c icalrecur_add_bydayrules
Version: 60 → Lightning 6.2

Do you have a timeline already on triaging and fixing this bug? By default we release the information after 30 days unless there are good reasons to delay the disclosure.

Magnus, can you find someone to see if this is fixed upstream and backport the patch?

Flags: needinfo?(mkmelin+mozilla)

Luis, I don't suppose you already had a patch for this too?

Ever confirmed: true

(In reply to Magnus Melin [:mkmelin] from comment #3)

Luis, I don't suppose you already had a patch for this too?

Nope, sorry. But maybe upstream? :)

Minusing for Mozilla bounty as Thunderbird and items relating to it are not part of our bounty program.

Flags: sec-bounty? → sec-bounty-
Assignee: nobody → geoff
Flags: needinfo?(mkmelin+mozilla)
Attached patch 1553808-1.diff β€” β€” Splinter Review

Ignoring the poor formatting in that function, I've copied the affected part from upstream.

Attachment #9069837 - Flags: review?(philipp)
Attachment #9069837 - Flags: approval-calendar-esr?(philipp)
Attachment #9069837 - Flags: approval-calendar-beta?(philipp)

Can you keep the poor formatting from upstream? This might make it easier to upgrade and see the differences when we import a newer libical version. Also, it seems the commit that added that has a bunch of other changes, can you check if we can import the whole changeset instead?

Flags: needinfo?(geoff)

The poor formatting isn't from upstream, it's ours. Upstream has tidied up.

The commit from upstream is +213 lines, -101 lines. I get that you want to stay vaguely in sync, but we're so far behind it's ridiculous.

Flags: needinfo?(geoff)
Comment on attachment 9069837 [details] [diff] [review]

Review of attachment 9069837 [details] [diff] [review]:

I guess you have a point there :) Go ahead then!
Attachment #9069837 - Flags: review?(philipp)
Attachment #9069837 - Flags: review+
Attachment #9069837 - Flags: approval-calendar-esr?(philipp)
Attachment #9069837 - Flags: approval-calendar-esr+
Attachment #9069837 - Flags: approval-calendar-beta?(philipp)
Attachment #9069837 - Flags: approval-calendar-beta+
Blocks: 1557562
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 7.1
Alias: CVE-2019-11705
Group: mail-core-security → core-security-release
Flags: sec-bounty-hof+
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.