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 (heap-corruption-submit.eml) in Thunderbird. Receiving this message in my inbox also triggers the bug without further user interaction. The thunderbird process is killed.

Triggering the bug might require several attempts, due to differences in the heap state. See crash reports attached.

Actual results:

This bug manifests with several effects, including out of bounds read, write, null pointer dereference and heap corruption, depending on the heap state at the moment of the overflow.

The vulnerable function icalmemory_strdup_and_dequote() lacks proper bounds checking while looping through an input buffer, which can be controlled by a remote user. When an unexpected input is provided, the loops goes out of bound causing out of bounds reads in the input buffer and out of bounds writes to a heap allocated buffer.

$ gdb --args thunderbird heap-corruption-submit.eml
[...]
Thread 1 "thunderbird" received signal SIGSEGV, Segmentation fault.
0x0000555555561cae in malloc ()

Reports for read, write, nullptr deref and heap corruption:

==2337==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62d000009b84 at pc 0x00000053619f bp 0x7fff694798b0 sp 0x7fff694798a8
READ of size 1 at 0x62d000009b84 thread T0
#0 0x53619e in icalmemory_strdup_and_dequote /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalvalue.c:202:19
#1 0x5355c2 in icalvalue_new_from_string_with_error /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalvalue.c:546:27
#2 0x519251 in icalparser_add_line /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:1075:14
#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 0x7f2c4823bb96 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)

0x62d000009b84 is located 0 bytes to the right of 38788-byte region [0x62d000000400,0x62d000009b84)
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 0x516f03 in make_segment /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:224:11
#3 0x519195 in icalparser_get_value /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:327:11
#4 0x519195 in icalparser_add_line /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:1058
#5 0x517e1b in icalparser_parse /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:623:11
#6 0x4fd243 in icalparser_parse_string /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:1236:9
#7 0x4fa975 in LLVMFuzzerTestOneInput (/opt/libfuzzer/thunderbird_libical_fuzzer+0x4fa975)
#8 0x43a681 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/opt/libfuzzer/thunderbird_libical_fuzzer+0x43a681)
#9 0x424327 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/opt/libfuzzer/thunderbird_libical_fuzzer+0x424327)
#10 0x42a4c1 in fuzzer::FuzzerDriver(int*, char***, int ()(unsigned char const, unsigned long)) (/opt/libfuzzer/thunderbird_libical_fuzzer+0x42a4c1)
#11 0x453f62 in main (/opt/libfuzzer/thunderbird_libical_fuzzer+0x453f62)
#12 0x7f2c4823bb96 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/icalvalue.c:202:19 in icalmemory_strdup_and_dequote

==2357==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000000206 at pc 0x000000536197 bp 0x7ffc511418b0 sp 0x7ffc511418a8
WRITE of size 1 at 0x603000000206 thread T0
#0 0x536196 in icalmemory_strdup_and_dequote /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalvalue.c
#1 0x5355c2 in icalvalue_new_from_string_with_error /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalvalue.c:546:27
#2 0x519251 in icalparser_add_line /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:1075:14
#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 0x7fc553950b96 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)

0x603000000206 is located 0 bytes to the right of 22-byte region [0x6030000001f0,0x603000000206)
allocated by thread T0 here:
#0 0x4cbb5d in malloc (/opt/libfuzzer/thunderbird_libical_fuzzer+0x4cbb5d)
#1 0x535eae in icalmemory_strdup_and_dequote /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalvalue.c:193:24
#2 0x5355c2 in icalvalue_new_from_string_with_error /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalvalue.c:546:27
#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 0x7fc553950b96 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/icalvalue.c in icalmemory_strdup_and_dequote

==10900==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fa1b13fbd45 bp 0x7fff1c4e7ec0 sp 0x7fff1c4e7368 T10900)

thunderbird_libical_fuzzer: malloc.c:2401: sysmalloc: Assertion `(old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)' failed.
==10928== ERROR: libFuzzer: deadly signal
#0 0x45c5b0 in __sanitizer_print_stack_trace (/opt/libfuzzer-noasan/thunderbird_libical_fuzzer+0x45c5b0)
#1 0x43c61b in fuzzer::PrintStackTrace() (/opt/libfuzzer-noasan/thunderbird_libical_fuzzer+0x43c61b)
#2 0x422083 in fuzzer::Fuzzer::CrashCallback() (/opt/libfuzzer-noasan/thunderbird_libical_fuzzer+0x422083)
#3 0x7f0d4197f88f (/lib/x86_64-linux-gnu/libpthread.so.0+0x1288f)
#4 0x7f0d40bf8e96 in __libc_signal_restore_set /build/glibc-OTsEL5/glibc-2.27/signal/../sysdeps/unix/sysv/linux/nptl-signals.h:80
#5 0x7f0d40bf8e96 in raise /build/glibc-OTsEL5/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:48
#6 0x7f0d40bfa800 in abort /build/glibc-OTsEL5/glibc-2.27/stdlib/abort.c:79
#7 0x7f0d40c4da90 in __malloc_assert /build/glibc-OTsEL5/glibc-2.27/malloc/malloc.c:298
#8 0x7f0d40c4da90 in sysmalloc /build/glibc-OTsEL5/glibc-2.27/malloc/malloc.c:2398
#9 0x7f0d40c4efef in _int_malloc /build/glibc-OTsEL5/glibc-2.27/malloc/malloc.c:4125
#10 0x7f0d40c512ec in malloc /build/glibc-OTsEL5/glibc-2.27/malloc/malloc.c:3065
#11 0x479131 in pvl_new_element /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/pvl.c:118:36
#12 0x47930f in pvl_push /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/pvl.c:194:28
#13 0x46c7e7 in icalproperty_add_parameters /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalproperty.c:87:6
#14 0x48c215 in icalproperty_vanew_xlicerror /opt/src/thunderbird-60.6.1/obj-x86_64-pc-linux-gnu/comm/calendar/libical/src/libical/icalderivedproperty.c:3214:4
#15 0x46b240 in insert_error /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:584:3
#16 0x46ae75 in icalparser_add_line /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:883:3
#17 0x46a3fd in icalparser_parse /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:623:11
#18 0x45de8a in icalparser_parse_string /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:1236:9
#19 0x45c8f4 in LLVMFuzzerTestOneInput (/opt/libfuzzer-noasan/thunderbird_libical_fuzzer+0x45c8f4)
#20 0x4235a1 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/opt/libfuzzer-noasan/thunderbird_libical_fuzzer+0x4235a1)
#21 0x40d247 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/opt/libfuzzer-noasan/thunderbird_libical_fuzzer+0x40d247)
#22 0x4133e1 in fuzzer::FuzzerDriver(int*, char***, int ()(unsigned char const, unsigned long)) (/opt/libfuzzer-noasan/thunderbird_libical_fuzzer+0x4133e1)
#23 0x43ce82 in main (/opt/libfuzzer-noasan/thunderbird_libical_fuzzer+0x43ce82)
#24 0x7f0d40bdbb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
#25 0x406b69 in _start (/opt/libfuzzer-noasan/thunderbird_libical_fuzzer+0x406b69)

free(): invalid next size (fast)
==10502== ERROR: libFuzzer: deadly signal
#0 0x45c5b0 in __sanitizer_print_stack_trace (/opt/libfuzzer-noasan/thunderbird_libical_fuzzer+0x45c5b0)
#1 0x43c61b in fuzzer::PrintStackTrace() (/opt/libfuzzer-noasan/thunderbird_libical_fuzzer+0x43c61b)
#2 0x422083 in fuzzer::Fuzzer::CrashCallback() (/opt/libfuzzer-noasan/thunderbird_libical_fuzzer+0x422083)
#3 0x7f40bfdd888f (/lib/x86_64-linux-gnu/libpthread.so.0+0x1288f)
#4 0x7f40bf051e96 in __libc_signal_restore_set /build/glibc-OTsEL5/glibc-2.27/signal/../sysdeps/unix/sysv/linux/nptl-signals.h:80
#5 0x7f40bf051e96 in raise /build/glibc-OTsEL5/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:48
#6 0x7f40bf053800 in abort /build/glibc-OTsEL5/glibc-2.27/stdlib/abort.c:79
#7 0x7f40bf09c896 in __libc_message /build/glibc-OTsEL5/glibc-2.27/libio/../sysdeps/posix/libc_fatal.c:181
#8 0x7f40bf0a3909 in malloc_printerr /build/glibc-OTsEL5/glibc-2.27/malloc/malloc.c:5350
#9 0x7f40bf0aaf5f in _int_free /build/glibc-OTsEL5/glibc-2.27/malloc/malloc.c:4213
#10 0x7f40bf0aaf5f in free /build/glibc-OTsEL5/glibc-2.27/malloc/malloc.c:3124
#11 0x45e3f9 in icalproperty_free /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalproperty.c:253:2
#12 0x45e224 in icalcomponent_free /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalcomponent.c:262:6
#13 0x469d5b in icalparser_free /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:170:2
#14 0x45def4 in icalparser_parse_string /opt/src/thunderbird-60.6.1/comm/calendar/libical/src/libical/icalparser.c:1240:5
#15 0x45c8f4 in LLVMFuzzerTestOneInput (/opt/libfuzzer-noasan/thunderbird_libical_fuzzer+0x45c8f4)
#16 0x4235a1 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/opt/libfuzzer-noasan/thunderbird_libical_fuzzer+0x4235a1)
#17 0x40d247 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/opt/libfuzzer-noasan/thunderbird_libical_fuzzer+0x40d247)
#18 0x4133e1 in fuzzer::FuzzerDriver(int*, char***, int ()(unsigned char const, unsigned long)) (/opt/libfuzzer-noasan/thunderbird_libical_fuzzer+0x4133e1)
#19 0x43ce82 in main (/opt/libfuzzer-noasan/thunderbird_libical_fuzzer+0x43ce82)
#20 0x7f40bf034b96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
#21 0x406b69 in _start (/opt/libfuzzer-noasan/thunderbird_libical_fuzzer+0x406b69)

Expected results:

No heap overflow nor corruption.

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

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 libical upstream?

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

--- comm/calendar/libical/src/libical/icalvalue.c       2019-05-28 09:04:23.443913362 +0000
+++ comm/calendar/libical/src/libical/icalvalue.c       2019-05-28 09:08:47.834598346 +0000
@@ -189,77 +189,80 @@

 static char* icalmemory_strdup_and_dequote(const char* str)
 {
-    const char* p;
-    char* out = (char*)malloc(sizeof(char) * strlen(str) +1);
-    char* pout;
+    const char *p;
+    char *out = (char *)malloc(sizeof(char) * strlen(str) + 1);
+    char *pout;
+    int wroteNull = 0;

-    if (out == 0){
-       return 0;
+    if (out == 0) {
+        return 0;
     }

     pout = out;

-    for (p = str; *p!=0; p++){
-
-       if( *p == '\\')
-       {
-           p++;
-           switch(*p){
-               case 0:
-               {
-                   *pout = '\0';
-                   break;
-
-               }
-               case 'n':
-               case 'N':
-               {
-                   *pout = '\n';
-                   break;
-               }
-               case 't':
-               case 'T':
-               {
-                   *pout = '\t';
-                   break;
-               }
-               case 'r':
-               case 'R':
-               {
-                   *pout = '\r';
-                   break;
-               }
-               case 'b':
-               case 'B':
-               {
-                   *pout = '\b';
-                   break;
-               }
-               case 'f':
-               case 'F':
-               {
-                   *pout = '\f';
-                   break;
-               }
-               case ';':
-               case ',':
-               case '"':
-               case '\\':
-               {
-                   *pout = *p;
-                   break;
-               }
-               default:
-               {
-                   *pout = ' ';
-               }
-           }
-       } else {
-           *pout = *p;
-       }         
+    /* Stop the loop when encountering a terminator in the source string                            
+       or if a null has been written to the destination. This prevents
+       reading past the end of the source string if the last character
+       is a backslash. */
+    for (p = str; !wroteNull && *p != 0; p++) {
+                     
+        if (*p == '\\') {
+            p++;                 
+            switch (*p) { 
+            case 0:
+                {             
+                    wroteNull = 1;      //stops iteration so p isn't incremented past the end of str
+                    *pout = '\0';
+                    break;       
+                }         
+            case 'n': 
+            case 'N': 
+                {    
+                    *pout = '\n';
+                    break;       
+                }         
+            case 't':
+            case 'T':   
+                {       
+                    *pout = '\t';
+                    break;       
+                }         
+            case 'r':
+            case 'R':   
+                {       
+                    *pout = '\r';
+                    break;       
+                }         
+            case 'b':
+            case 'B':   
+                {       
+                    *pout = '\b';
+                    break;      
+                }        
+            case 'f':          
+            case 'F':     
+                {       
+                    *pout = '\f';
+                    break;      
+                }               
+            case ';':
+            case ',':   
+            case '"':   
+            case '\\': 
+                {               
+                    *pout = *p;
+                    break;
+                }       
+            default:    
+                {       
+                    *pout = ' ';
+                }
+            }                 
+        } else {         
+            *pout = *p;
+        }              
-       pout++;                 
-                
+        pout++;                      
     }          
                                 
     *pout = '\0';
@@ -267,6 +270,8 @@                                                      
     return out;                                                       
 }                                                                     
                          
+                                               
+
  /*                      
   * Returns a quoted copy of a string
  */                      

Could you attach it as a patch? Use the create attachment link. (Same for the other bug)

(In reply to luis.merino from comment #1)

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.

The next regularly scheduled release won't be until at least 2019-07-09 60.8.0, unless we do an extra point release 60.7.1

That's quite an old patch :-( https://github.com/libical/libical/commit/a4230eb8bda035016861d69fe5d29705b737c9de

(In reply to Wayne Mery (:wsmwk) from comment #5)

The next regularly scheduled release won't be until at least 2019-07-09 60.8.0, unless we do an extra point release 60.7.1

Please, keep in mind this issue is easy to find via fuzzing, reachable remotely and most probably exploitable without user interaction. The patch has been out for years and even a corpus triggering it was published and reported 3 years ago (see https://bugzilla.mozilla.org/show_bug.cgi?id=1275400). This also applies to some extent to the other issues reported recently (see #1555646, #1553814 and #1553808).

This said, I would recommend doing a release as soon as possible.

Made it a proper hg patch

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

(In reply to Philipp Kewisch [:Fallen] [:📆] from comment #10)

Can you find and link all the commits involved here? It seems more than the
one from comment 7.

I think the commit from comment 7 is all. The rest is just changed because they did some code reformatting, but no functional changes: https://github.com/libical/libical/blame/master/src/libical/icalvalue.c#L192

https://hg.mozilla.org/comm-central/rev/def9f7f3af210dc5ec4474802cded6c249ee4166
Heap buffer overflow in icalvalue.c icalmemory_strdup_and_dequote. r=philipp

Are you requesting CVE for this or we do?

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

https://hg.mozilla.org/comm-central/rev/def9f7f3af210dc5ec4474802cded6c249ee4166
Heap buffer overflow in icalvalue.c icalmemory_strdup_and_dequote. 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.

(In reply to luis.merino from comment #14)

Are you requesting CVE for this or we do?

Normally we'd expect a bug in iCal to get a CVE from iCal, but since this is long patched upstream we could issue one for "Thunderbird's implementation of iCal". Those are usually assigned during the release/advisory-writing process and will be added to this bug. Or you can email security@mozilla.org about it if you can't wait.

Based on comment 16, I'll send email to Mozilla and request CVE IDs.

TB 68 beta / Cal 7.0:
https://hg.mozilla.org/releases/comm-beta/rev/671757465ce376c9b238b1969c7fe2d6f5489386

TB 60.7.1 ESR / Cal 6.2.7.1:
https://hg.mozilla.org/releases/comm-esr60/rev/58768479ed77a3cf96967140ad1b9781a9670bac