Closed Bug 1553820 (CVE-2019-11703) Opened 1 year ago Closed 1 year ago

Heap buffer overflow in icalparser.c parser_get_next_char

Categories

(Calendar :: General, defect)

Lightning 6.2
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: luis.merino, Assigned: luis.merino)

References

Details

(Keywords: csectype-bounds, sec-high)

Attachments

(3 files, 2 obsolete files)

Attached file heap-read β€”

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

Steps to reproduce:

While fuzzing icalparser_parse_string from Thunderbird's fork of libical built with ASAN, a heap buffer overflow read triggers in parser_get_next_char with some inputs. A reliable reproducer is attached.

Actual results:

==2528==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60700000040f at pc 0x00000052c655 bp 0x7ffef533cc30 sp 0x7ffef533cc28
READ of size 1 at 0x60700000040f thread T0
#0 0x52c654 in parser_get_next_char /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:208:38
#1 0x5183bc in parser_get_prop_name /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:247:9
#2 0x5183bc in icalparser_add_line /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:700
#3 0x517e1b in icalparser_parse /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:623:11
#4 0x4fd243 in icalparser_parse_string /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:1236:9
#5 0x4fa975 in LLVMFuzzerTestOneInput (/opt/libfuzzer/thunderbird_libical_fuzzer+0x4fa975)
#6 0x43a681 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/opt/libfuzzer/thunderbird_libical_fuzzer+0x43a681)
#7 0x424327 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/opt/libfuzzer/thunderbird_libical_fuzzer+0x424327)
#8 0x42a4c1 in fuzzer::FuzzerDriver(int*, char***, int ()(unsigned char const, unsigned long)) (/opt/libfuzzer/thunderbird_libical_fuzzer+0x42a4c1)
#9 0x453f62 in main (/opt/libfuzzer/thunderbird_libical_fuzzer+0x453f62)
#10 0x7f28bc086b96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
#11 0x41dc49 in _start (/opt/libfuzzer/thunderbird_libical_fuzzer+0x41dc49)

0x60700000040f is located 1 bytes to the left of 80-byte region [0x607000000410,0x607000000460)
allocated by thread T0 here:
#0 0x4cbb5d in malloc (/opt/libfuzzer/thunderbird_libical_fuzzer+0x4cbb5d)
#1 0x4fe7bd in icalmemory_new_buffer /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalmemory.c:266:15
#2 0x51719b in icalparser_get_line /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:453:21
#3 0x517e0d in icalparser_parse /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:621:13
#4 0x4fd243 in icalparser_parse_string /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:1236:9
#5 0x4fa975 in LLVMFuzzerTestOneInput (/opt/libfuzzer/thunderbird_libical_fuzzer+0x4fa975)
#6 0x43a681 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/opt/libfuzzer/thunderbird_libical_fuzzer+0x43a681)
#7 0x424327 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/opt/libfuzzer/thunderbird_libical_fuzzer+0x424327)
#8 0x42a4c1 in fuzzer::FuzzerDriver(int*, char***, int ()(unsigned char const, unsigned long)) (/opt/libfuzzer/thunderbird_libical_fuzzer+0x42a4c1)
#9 0x453f62 in main (/opt/libfuzzer/thunderbird_libical_fuzzer+0x453f62)
#10 0x7f28bc086b96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310

SUMMARY: AddressSanitizer: heap-buffer-overflow /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:208:38 in parser_get_next_char

Expected results:

No heap buffer overflow.

When the vulnerable function in Thunderbird is replaced with current implementation in libical upstream (https://github.com/libical/libical/blob/master/src/libical/icalparser.c#L147) this bug doesn't manifest anymore.

Flags: sec-bounty?
Component: Untriaged → General
Product: Thunderbird → Calendar
Summary: Heap buffer overflow in parser_get_next_char → Heap buffer overflow in icalparser.c parser_get_next_char
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 backport the relevant patches from upstream?

Flags: needinfo?(mkmelin+mozilla)

I've been using this patch to apply parser_get_next_char from latest upstream:

--- comm/calendar/libical/src/libical/icalparser.c      2019-05-28 09:12:25.577672451 +0000
+++ comm/calendar/libical/src/libical/icalparser.c      2019-05-28 09:15:01.604897470 +0000
@@ -190,25 +190,24 @@
 char* parser_get_next_char(char c, char *str, int qm)
 {
     int quote_mode = 0;
-    char* p;
+    char *p = str;
+    char next_char = *p;
+    char prev_char = 0;
 
-    for(p=str; *p!=0; p++){
-           if (qm == 1) {
-                               if ( quote_mode == 0 && *p=='"' && *(p-1) != '\\' ){
-                                               quote_mode =1;
-                                               continue;
-                               }
-
-                               if ( quote_mode == 1 && *p=='"' && *(p-1) != '\\' ){
-                                               quote_mode =0;
-                                               continue;
-                               }
-           }
-
-               if (quote_mode == 0 &&  *p== c  && *(p-1) != '\\' ){
-                               return p;
-               } 
+    while (next_char != '\0') {
+        if ((prev_char != '\0') && (prev_char != '\\')) {
+            if (qm == 1 && next_char == '"') {
+                /* Encountered a quote, toggle quote mode */
+                quote_mode = !quote_mode;
+            } else if (quote_mode == 0 && next_char == c) {
+                /* Found a matching character out of quote mode, return it */
+                return p;
+            }
+        }
 
+        /* Save the previous character so we can check if it's a backslash in the next iteration */
+        prev_char = next_char;
+        next_char = *(++p);
     }
 
     return 0;
Attached patch parser_get_next_char.patch (obsolete) β€” β€” Splinter Review

calling sec-high assuming this can be triggered in Thunderbird from an .ics mail attachment, but I haven't tried to see if the attached POC can be made to do that.

Assignee: nobody → luis.merino
Flags: needinfo?(mkmelin+mozilla)
Attached patch bug1553808_libical_icalrecur.patch (obsolete) β€” β€” Splinter Review
Attachment #9068015 - Attachment is obsolete: true
Attachment #9069087 - Flags: review?(philipp)
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 9069087 [details] [diff] [review]
bug1553808_libical_icalrecur.patch

Review of attachment 9069087 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. For the record, this seems to be https://github.com/libical/libical/commit/9a6b85328d6b661ddb54f5b6433edb2397abc130
Attachment #9069087 - Flags: review?(philipp) → review+

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

Flags: sec-bounty? → sec-bounty-

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

Made that a proper hg patch.

It would have been nice if you had provided a commit message relating to this bug here and not bug 1553808 :-(

https://hg.mozilla.org/comm-central/rev/383efc5887ecf146ecfb8f5e5853b5869242e27b
Heap buffer overflow in icalparser.c parser_get_next_char. r=philipp

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 7.1

https://hg.mozilla.org/comm-central/rev/f4628616dd10a3b267b6c2c9540b7492ab64f6fa
Backed out changeset 383efc5887ec (bug 1553820) for failure in test_bug1209399.js. a=backout DONTBUILD

Status: RESOLVED → REOPENED
Flags: needinfo?(philipp)
Flags: needinfo?(luis.merino)
Resolution: FIXED → ---
Target Milestone: 7.1 → ---

(In reply to Jorg K (GMT+2) from comment #11)

https://hg.mozilla.org/comm-central/rev/f4628616dd10a3b267b6c2c9540b7492ab64f6fa
Backed out changeset 383efc5887ec (bug 1553820) for failure in test_bug1209399.js. a=backout DONTBUILD

Sadly, nothing to say here.

Flags: needinfo?(luis.merino)

Someone needs to debug why the test fails with the patch.

I wonder if the difference between "" and null even matters here. Philipp?

(In reply to Geoff Lankow (:darktrojan) from comment #14)

I wonder if the difference between "" and null even matters here. Philipp?

I'm guessing the check used to be ===, but equal now just does ==, so no difference at the moment.

I'm assuming Magnus or Geoff have the engineering work here covered, clearing my NI.

Flags: needinfo?(philipp)

(In reply to Jorg K (GMT+2) from comment #10)

https://hg.mozilla.org/comm-central/rev/383efc5887ecf146ecfb8f5e5853b5869242e27b
Heap buffer overflow in icalparser.c parser_get_next_char. r=philipp

Since the patch is already public with references to this bug, we will publish a short writeup in the next days.

Many thanks for addressing this so quickly.

Attached patch 1553820-part1-1.diff β€” β€” Splinter Review

There seems to be a bug in libical that prevents things happening on the first character in str. (Compare this patch to the last.) I'm not imagining that, am I?

Luis, can you check this patch still solves the reported problem? I think it should be fine, but just to be sure…

Attachment #9069087 - Attachment is obsolete: true
Attachment #9070118 - Flags: feedback?(mkmelin+mozilla)
Attachment #9070118 - Flags: feedback?(luis.merino)
Comment on attachment 9070118 [details] [diff] [review]
1553820-part1-1.diff

Review of attachment 9070118 [details] [diff] [review]:
-----------------------------------------------------------------

Looks correct to me. Maybe we should also submit that upstream?
Attachment #9070118 - Flags: feedback?(mkmelin+mozilla) → feedback+

I will, but I wanted to check I was actually right first.

Edit: https://github.com/libical/libical/pull/395

(In reply to Geoff Lankow (:darktrojan) from comment #17)

Created attachment 9070118 [details] [diff] [review]
1553820-1.diff

There seems to be a bug in libical that prevents things happening on the first character in str. (Compare this patch to the last.) I'm not imagining that, am I?

Luis, can you check this patch still solves the reported problem? I think it should be fine, but just to be sure…

After applying your patch, a new heap overflow reading out of bounds appears:

2019-06-06_13:24:24.85446 INFO:dasfuzzer.cpu1:==29877==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6070016e89cf at pc 0x00000051b285 bp 0x7ffdee9df010 sp 0x7ffdee9df008
2019-06-06_13:24:24.85458 INFO:dasfuzzer.cpu1:READ of size 1 at 0x6070016e89cf thread T0
2019-06-06_13:24:24.87828 INFO:dasfuzzer.cpu1:#0 0x51b284 in parser_get_next_value /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:385:19
2019-06-06_13:24:24.87841 INFO:dasfuzzer.cpu1:#1 0x51a6a5 in icalparser_add_line /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:1053:24
2019-06-06_13:24:24.87854 INFO:dasfuzzer.cpu1:#2 0x518df7 in icalparser_parse /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:622:11
2019-06-06_13:24:24.87866 INFO:dasfuzzer.cpu1:#3 0x4fe242 in icalparser_parse_string /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:1235:9
2019-06-06_13:24:24.87880 INFO:dasfuzzer.cpu1:#4 0x4fb9d5 in LLVMFuzzerTestOneInput /opt/src/fuzzer.cc:13:38
2019-06-06_13:24:24.87893 INFO:dasfuzzer.cpu1:#5 0x43b7c1 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/opt/out/thunderbird_libical_fuzzer+0x43b7c1)
2019-06-06_13:24:24.87916 INFO:dasfuzzer.cpu1:#6 0x43aef5 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) (/opt/out/thunderbird_libical_fuzzer+0x43aef5)
2019-06-06_13:24:24.87938 INFO:dasfuzzer.cpu1:#7 0x43d8fe in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/opt/out/thunderbird_libical_fuzzer+0x43d8fe)
2019-06-06_13:24:24.87950 INFO:dasfuzzer.cpu1:#8 0x43df00 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/opt/out/thunderbird_libical_fuzzer+0x43df00)
2019-06-06_13:24:24.87962 INFO:dasfuzzer.cpu1:#9 0x42aa89 in fuzzer::FuzzerDriver(int*, char***, int ()(unsigned char const, unsigned long)) (/opt/out/thunderbird_libical_fuzzer+0x42aa89)
2019-06-06_13:24:24.87973 INFO:dasfuzzer.cpu1:#10 0x455002 in main (/opt/out/thunderbird_libical_fuzzer+0x455002)
2019-06-06_13:24:24.92405 INFO:dasfuzzer.cpu1:#11 0x7f1c84f1cb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
2019-06-06_13:24:24.92431 INFO:dasfuzzer.cpu1:#12 0x41dc99 in _start (/opt/out/thunderbird_libical_fuzzer+0x41dc99)
2019-06-06_13:24:24.92436 INFO:dasfuzzer.cpu1:
2019-06-06_13:24:24.92440 INFO:dasfuzzer.cpu1:0x6070016e89cf is located 1 bytes to the left of 80-byte region [0x6070016e89d0,0x6070016e8a20)
2019-06-06_13:24:24.92443 INFO:dasfuzzer.cpu1:allocated by thread T0 here:
2019-06-06_13:24:24.92455 INFO:dasfuzzer.cpu1:#0 0x4cce6d in malloc (/opt/out/thunderbird_libical_fuzzer+0x4cce6d)
2019-06-06_13:24:24.92467 INFO:dasfuzzer.cpu1:#1 0x4ff7bc in icalmemory_new_buffer /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalmemory.c:266:15
2019-06-06_13:24:24.92480 INFO:dasfuzzer.cpu1:#2 0x518182 in icalparser_get_line /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:452:21
2019-06-06_13:24:24.92491 INFO:dasfuzzer.cpu1:#3 0x518de9 in icalparser_parse /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:620:13
2019-06-06_13:24:24.92503 INFO:dasfuzzer.cpu1:#4 0x4fe242 in icalparser_parse_string /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:1235:9
2019-06-06_13:24:24.92514 INFO:dasfuzzer.cpu1:#5 0x4fb9d5 in LLVMFuzzerTestOneInput /opt/src/fuzzer.cc:13:38
2019-06-06_13:24:24.92526 INFO:dasfuzzer.cpu1:#6 0x43b7c1 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/opt/out/thunderbird_libical_fuzzer+0x43b7c1)
2019-06-06_13:24:24.92537 INFO:dasfuzzer.cpu1:#7 0x43aef5 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) (/opt/out/thunderbird_libical_fuzzer+0x43aef5)
2019-06-06_13:24:24.92549 INFO:dasfuzzer.cpu1:#8 0x43d8fe in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/opt/out/thunderbird_libical_fuzzer+0x43d8fe)
2019-06-06_13:24:24.92560 INFO:dasfuzzer.cpu1:#9 0x43df00 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/opt/out/thunderbird_libical_fuzzer+0x43df00)
2019-06-06_13:24:24.92571 INFO:dasfuzzer.cpu1:#10 0x42aa89 in fuzzer::FuzzerDriver(int*, char***, int ()(unsigned char const, unsigned long)) (/opt/out/thunderbird_libical_fuzzer+0x42aa89)
2019-06-06_13:24:24.92582 INFO:dasfuzzer.cpu1:#11 0x455002 in main (/opt/out/thunderbird_libical_fuzzer+0x455002)
2019-06-06_13:24:24.92598 INFO:dasfuzzer.cpu1:#12 0x7f1c84f1cb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
2019-06-06_13:24:24.92601 INFO:dasfuzzer.cpu1:
2019-06-06_13:24:24.93552 INFO:dasfuzzer.cpu1:SUMMARY: AddressSanitizer: heap-buffer-overflow /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:385:19 in parser_get_next_value

Comment on attachment 9070118 [details] [diff] [review]
1553820-part1-1.diff

Review of attachment 9070118 [details] [diff] [review]:
-----------------------------------------------------------------

After applying your patch, a new heap overflow reading out of bounds appears:

2019-06-06_13:24:24.85446 INFO:dasfuzzer.cpu1:==29877==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6070016e89cf at pc 0x00000051b285 bp 0x7ffdee9df010 sp 0x7ffdee9df008
2019-06-06_13:24:24.85458 INFO:dasfuzzer.cpu1:READ of size 1 at 0x6070016e89cf thread T0
2019-06-06_13:24:24.87828 INFO:dasfuzzer.cpu1:#0 0x51b284 in parser_get_next_value /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:385:19
2019-06-06_13:24:24.87841 INFO:dasfuzzer.cpu1:#1 0x51a6a5 in icalparser_add_line /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:1053:24
2019-06-06_13:24:24.87854 INFO:dasfuzzer.cpu1:#2 0x518df7 in icalparser_parse /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:622:11
2019-06-06_13:24:24.87866 INFO:dasfuzzer.cpu1:#3 0x4fe242 in icalparser_parse_string /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:1235:9
2019-06-06_13:24:24.87880 INFO:dasfuzzer.cpu1:#4 0x4fb9d5 in LLVMFuzzerTestOneInput /opt/src/fuzzer.cc:13:38
2019-06-06_13:24:24.87893 INFO:dasfuzzer.cpu1:#5 0x43b7c1 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/opt/out/thunderbird_libical_fuzzer+0x43b7c1)
2019-06-06_13:24:24.87916 INFO:dasfuzzer.cpu1:#6 0x43aef5 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) (/opt/out/thunderbird_libical_fuzzer+0x43aef5)
2019-06-06_13:24:24.87938 INFO:dasfuzzer.cpu1:#7 0x43d8fe in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/opt/out/thunderbird_libical_fuzzer+0x43d8fe)
2019-06-06_13:24:24.87950 INFO:dasfuzzer.cpu1:#8 0x43df00 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/opt/out/thunderbird_libical_fuzzer+0x43df00)
2019-06-06_13:24:24.87962 INFO:dasfuzzer.cpu1:#9 0x42aa89 in fuzzer::FuzzerDriver(int*, char***, int ()(unsigned char const, unsigned long)) (/opt/out/thunderbird_libical_fuzzer+0x42aa89)
2019-06-06_13:24:24.87973 INFO:dasfuzzer.cpu1:#10 0x455002 in main (/opt/out/thunderbird_libical_fuzzer+0x455002)
2019-06-06_13:24:24.92405 INFO:dasfuzzer.cpu1:#11 0x7f1c84f1cb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
2019-06-06_13:24:24.92431 INFO:dasfuzzer.cpu1:#12 0x41dc99 in _start (/opt/out/thunderbird_libical_fuzzer+0x41dc99)
2019-06-06_13:24:24.92436 INFO:dasfuzzer.cpu1:
2019-06-06_13:24:24.92440 INFO:dasfuzzer.cpu1:0x6070016e89cf is located 1 bytes to the left of 80-byte region [0x6070016e89d0,0x6070016e8a20)
2019-06-06_13:24:24.92443 INFO:dasfuzzer.cpu1:allocated by thread T0 here:
2019-06-06_13:24:24.92455 INFO:dasfuzzer.cpu1:#0 0x4cce6d in malloc (/opt/out/thunderbird_libical_fuzzer+0x4cce6d)
2019-06-06_13:24:24.92467 INFO:dasfuzzer.cpu1:#1 0x4ff7bc in icalmemory_new_buffer /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalmemory.c:266:15
2019-06-06_13:24:24.92480 INFO:dasfuzzer.cpu1:#2 0x518182 in icalparser_get_line /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:452:21
2019-06-06_13:24:24.92491 INFO:dasfuzzer.cpu1:#3 0x518de9 in icalparser_parse /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:620:13
2019-06-06_13:24:24.92503 INFO:dasfuzzer.cpu1:#4 0x4fe242 in icalparser_parse_string /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:1235:9
2019-06-06_13:24:24.92514 INFO:dasfuzzer.cpu1:#5 0x4fb9d5 in LLVMFuzzerTestOneInput /opt/src/fuzzer.cc:13:38
2019-06-06_13:24:24.92526 INFO:dasfuzzer.cpu1:#6 0x43b7c1 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/opt/out/thunderbird_libical_fuzzer+0x43b7c1)
2019-06-06_13:24:24.92537 INFO:dasfuzzer.cpu1:#7 0x43aef5 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) (/opt/out/thunderbird_libical_fuzzer+0x43aef5)
2019-06-06_13:24:24.92549 INFO:dasfuzzer.cpu1:#8 0x43d8fe in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/opt/out/thunderbird_libical_fuzzer+0x43d8fe)
2019-06-06_13:24:24.92560 INFO:dasfuzzer.cpu1:#9 0x43df00 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/opt/out/thunderbird_libical_fuzzer+0x43df00)
2019-06-06_13:24:24.92571 INFO:dasfuzzer.cpu1:#10 0x42aa89 in fuzzer::FuzzerDriver(int*, char***, int ()(unsigned char const, unsigned long)) (/opt/out/thunderbird_libical_fuzzer+0x42aa89)
2019-06-06_13:24:24.92582 INFO:dasfuzzer.cpu1:#11 0x455002 in main (/opt/out/thunderbird_libical_fuzzer+0x455002)
2019-06-06_13:24:24.92598 INFO:dasfuzzer.cpu1:#12 0x7f1c84f1cb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
2019-06-06_13:24:24.92601 INFO:dasfuzzer.cpu1:
2019-06-06_13:24:24.93552 INFO:dasfuzzer.cpu1:SUMMARY: AddressSanitizer: heap-buffer-overflow /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:385:19 in parser_get_next_value
Attachment #9070118 - Flags: feedback?(luis.merino) → feedback-
Attached patch 1553820-part2-1.diff β€” β€” Splinter Review

I think this is right, but upstream libical still has the same code that we do, so I'm suspicious of this change.

Attachment #9070519 - Flags: review?(philipp)
Attachment #9070519 - Flags: feedback?(luis.merino)
Attachment #9070118 - Attachment description: 1553820-1.diff → 1553820-part1-1.diff
Attachment #9070118 - Attachment filename: 1553820-1.diff → 1553820-part1-1.diff
Comment on attachment 9070519 [details] [diff] [review]
1553820-part2-1.diff

Review of attachment 9070519 [details] [diff] [review]:
-----------------------------------------------------------------

Codewise this looks fine, r=me
Attachment #9070519 - Flags: review?(philipp) → review+
Comment on attachment 9070519 [details] [diff] [review]
1553820-part2-1.diff

Review of attachment 9070519 [details] [diff] [review]:
-----------------------------------------------------------------

The heap buffer overflow detected disappears with this patch
Attachment #9070519 - Flags: feedback?(luis.merino) → feedback+

Geoff, could you please clarify the status of each of the attached patches?
I don't understand which patches need to be checked in.

The first was checked in, but backed out.

The second doesn't have review.

The third has review.

Could you please mark patches as "obsolete", that shouldn't get checked in?
Could you please get review on the patches that need to get checked in?

Flags: needinfo?(geoff)

ok, the first one isn't a patch, we have only two patches.

What needs to get checked in?

Also, please request approval for esr and beta for the patches that need to be uplifted.

Comment on attachment 9070519 [details] [diff] [review]
1553820-part2-1.diff

What about part 1?
Attachment #9070519 - Flags: approval-calendar-esr?(philipp)
Attachment #9070519 - Flags: approval-calendar-beta?(philipp)
Comment on attachment 9070118 [details] [diff] [review]
1553820-part1-1.diff

Sorry. Both patches should be checked in. This one effectively has review already.
Flags: needinfo?(geoff)
Attachment #9070118 - Flags: review+
Attachment #9070118 - Flags: approval-calendar-esr?(philipp)
Attachment #9070118 - Flags: approval-calendar-beta?(philipp)
Blocks: 1557562
Attachment #9070118 - Flags: approval-calendar-esr?(philipp)
Attachment #9070118 - Flags: approval-calendar-esr+
Attachment #9070118 - Flags: approval-calendar-beta?(philipp)
Attachment #9070118 - Flags: approval-calendar-beta+
Attachment #9070519 - Flags: approval-calendar-esr?(philipp)
Attachment #9070519 - Flags: approval-calendar-esr+
Attachment #9070519 - Flags: approval-calendar-beta?(philipp)
Attachment #9070519 - Flags: approval-calendar-beta+
Status: REOPENED → RESOLVED
Closed: 1 year ago1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 7.1
Alias: CVE-2019-11703
Duplicate of this bug: CVE-2016-5826

I'd be interested why this bug is bounty eligible now, but it wasn't three years ago when I reported it (as well as the handful of others I reported at the same time).

Hi Brandon. IIUC there isn't a bug bounty for Thunderbird issues.

It's been flagged as sec-bounty?

I see above Thunderbird is indeed not. Thanks for the clarification.

If there would be a bounty, the bounty should definitely go to Brandon for the bugs he reported three years ago. Especially if they were reported at a time where the bounty for TB was still in effect?

Flags: needinfo?(kaie)
Flags: needinfo?(bperry.volatile)

Per this comment by abillings 3 years ago, the reason these were not bountied is because they marked them as sec-low. Not because TB wasn't under the bounty.

https://bugzilla.mozilla.org/show_bug.cgi?id=1280832#c24

Now that we have decided they are indeed high-severity, I'd like to see these reconsidered.

Flags: needinfo?(bperry.volatile)

The silence is deafening.

Flags: needinfo?(dveditz)
Flags: needinfo?(abillings)

(In reply to Brandon Perry from comment #41)

The silence is deafening.

Leaving a comment here without a ni? is unlikely to be seen quickly. I hadn't seen your previous comment until you set the ni? today. (Many of us get 100s of emails for bugs a day.)

If you email security@mozilla.org, you can be guaranteed a quicker response. I'll mark this for bounty consideration in the meantime.

Flags: sec-bounty?
Flags: sec-bounty-
Flags: needinfo?(abillings)

In comment 5 I guessed "sec-high assuming this can be triggered in Thunderbird from an .ics mail attachment". Has that been tested/demonstrated? We called bug 1281041 sec-low because we thought not, so which is right?

Flags: needinfo?(dveditz) → needinfo?(bperry.volatile)

(In reply to Daniel Veditz [:dveditz] from comment #43)

In comment 5 I guessed "sec-high assuming this can be triggered in Thunderbird from an .ics mail attachment". Has that been tested/demonstrated? We called bug 1281041 sec-low because we thought not, so which is right?

This ical file will crash an ASAN enabled Thunderbird and given the right conditions (heap layout needs to be massaged) also production Thunderbird:

https://github.com/x41sec/advisories/blob/master/X41-2019-002/x41-2019-002-thunderbird.ical

Flags: needinfo?(kaie)
Group: mail-core-security → core-security-release
Flags: needinfo?(bperry.volatile)

Awarding a bounty for this since when it was first reported in bug 1281041 Thunderbird was covered by the bounty program. Splitting the bounty between the two bugs.

Flags: sec-bounty? → sec-bounty+

I appreciate the reconsideration. I'll be donating the bounty to Austin Pets Alive.

Group: core-security-release
You need to log in before you can comment on or make changes to this bug.