From: Thomas Roessler Date: Wed, 26 Aug 1998 19:13:33 +0000 (+0000) Subject: [patch-0.94.4i.tlr.rfc822_leak.1] Fixing a memory leak in X-Git-Tag: mutt-0-94-5i-rel~24 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=17e73c0ac6d9d00b60940bec7baa9a728ffa1a9c;p=mutt [patch-0.94.4i.tlr.rfc822_leak.1] Fixing a memory leak in the rfc822_parse_adrlist(). Some explanations seem to be in order here. Let's look at the code: 386 else if (*s == ';') 387 { 388 if (phraselen) 389 { 390 phrase[phraselen] = 0; 391 add_addrspec (&top, &last, phrase, comment, &commentlen, sizeof (comment) - 1); 392 } 393 else if (commentlen && !last->personal) 394 { 395 comment[commentlen] = 0; 396 last->personal = safe_strdup (comment); 397 } 398 #ifdef EXACT_ADDRESS 399 if (last && !last->val) Line 399 contains the change; previously, it looked like this: 399' if (last) 400 last->val = mutt_substrdup (begin, s); 401 #endif 402 403 /* add group terminator */ 404 cur = rfc822_new_address (); 405 if (last) 406 { 407 last->next = cur; 408 last = cur; 409 } 410 411 phraselen = 0; 412 commentlen = 0; 413 s++; 414 begin = s; 415 SKIPWS (begin); 416 } OK, what happens? There are essentially two situations here: -> We have already parsed a complete address specification and know about this fact, but there was no new address information. This is the case if we are parsing through addresses like undisclosed-recipients:; or recipients: a, b, c,; (Note the extra ',' before the ';'!) In this case, some of the other code in rfc822.c has already filled in last->val, and we really shouldn't overwrite that with a NULL pointer. -> The ';' finishes an address spec, like in recipients: a; In this case, last is either set by add_addrspec(), or it has already been set by some of the previous code (comment handling, ...). Anyway, last->val is still NULL, so it is correct to write the complete addr spec to last->val. --- diff --git a/TODO b/TODO index 11b0c84b..0f1dd525 100644 --- a/TODO +++ b/TODO @@ -15,63 +15,6 @@ I'm listing the problems in approximate order of priority. John Prevost wrote something on this on Aug 19). -- When using EXACT_ADDRESS, the following memory leak may - occur (insure++ output): - -| [rfc822.c:400] **LEAK_ASSIGN** -| >> last->val = mutt_substrdup (begin, s); -| -| Memory leaked due to pointer reassignment: (void *) malloc(siz) -| -| Lost block : 0x082e7da0 thru 0x082e7db6 (23 bytes) -| (void *) malloc(siz), allocated at: -| safe_malloc() lib.c, 283 -| mutt_substrdup() lib.c, 861 -| rfc822_parse_adrlist() rfc822.c, 377 -| mutt_read_rfc822_header() parse.c, 1112 -| mbox_parse_mailbox() mbox.c, 272 -| mbox_open_mailbox() mbox.c, 390 -| mx_open_mailbox() mx.c, 598 -| mutt_index_menu() curs_main.c, 874 -| -| Stack trace where the error occurred: -| rfc822_parse_adrlist() rfc822.c, 400 -| mutt_read_rfc822_header() parse.c, 1112 -| mbox_parse_mailbox() mbox.c, 272 -| mbox_open_mailbox() mbox.c, 390 -| mx_open_mailbox() mx.c, 598 -| mutt_index_menu() curs_main.c, 874 -| main() main.c, 635 -| -| ************************** INSURE SUMMARY ************************* v4.1 ** -| * Program : mutt * -| * Arguments : * -| * Directory : /home/olga/Mail * -| * Compiled on : Aug 18, 1998 19:52:01 * -| * Run on : Aug 18, 1998 19:57:08 * -| * Elapsed time : 00:04:26 * -| * Malloc HWM : 855,258 bytes (835K) * -| *************************************************************************** -| -| PROBLEM SUMMARY - by type -| =============== -| -| Problem Reported Suppressed -| ------------------------------------------------- -| LEAK_ASSIGN 1 1 -| RETURN_FAILURE 0 70 -| USER_ERROR 0 24 -| ------------------------------------------------- -| TOTAL 1 95 -| ------------------------------------------------- -| -| PROBLEM SUMMARY - by location -| =============== -| -| LEAK_ASSIGN: Memory leaked due to pointer reassignment, 1 unique occurrence -| 2 at rfc822.c, 400 -| - - In the "attachment" menu, assume this: 1 [text/plain, 7bit, 1.1K] diff --git a/rfc822.c b/rfc822.c index 2c990486..ce3c126f 100644 --- a/rfc822.c +++ b/rfc822.c @@ -396,7 +396,7 @@ ADDRESS *rfc822_parse_adrlist (ADDRESS *top, const char *s) last->personal = safe_strdup (comment); } #ifdef EXACT_ADDRESS - if (last) + if (last && !last->val) last->val = mutt_substrdup (begin, s); #endif