From d161aa539a449cc6fa66fb73ad54f997bcd49b13 Mon Sep 17 00:00:00 2001 From: Mattias Andrée Date: Fri, 2 Apr 2021 22:47:53 +0200 Subject: Fix parsing and memory leaks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Mattias Andrée --- common.h | 22 ++++++++++++--- libcontacts_address_destroy.c | 2 +- libcontacts_block_destroy.c | 2 +- libcontacts_chat_destroy.c | 2 +- libcontacts_contact_destroy.c | 27 ++++++++++-------- libcontacts_email_destroy.c | 2 +- libcontacts_load_contacts.c | 2 +- libcontacts_number_destroy.c | 2 +- libcontacts_organisation_destroy.c | 2 +- libcontacts_parse_contact.c | 57 +++++++++++++++++++++++++------------- libcontacts_pgpkey_destroy.c | 2 +- libcontacts_site_destroy.c | 2 +- 12 files changed, 79 insertions(+), 45 deletions(-) diff --git a/common.h b/common.h index d74dde5..ab021cc 100644 --- a/common.h +++ b/common.h @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -16,12 +17,25 @@ #define TIME_MAX ((time_t)((1ULL << (8 * sizeof(time_t) - 1)) - 1ULL)) -#define DESTROY_ALL(LIST, FUNC)\ +#define DESTROY_ALL_OBJECTS(LIST, FUNC)\ do {\ - void *destroy_all_temp__;\ - if ((destroy_all_temp__ = (LIST))) {\ - for (; *(LIST); (LIST)++)\ + void *destroy_all_temp__ = (LIST);\ + if (destroy_all_temp__) {\ + for (; *(LIST); (LIST)++) {\ FUNC(*(LIST));\ + free(*(LIST));\ + }\ + free(destroy_all_temp__);\ + }\ + } while (0) + + +#define DESTROY_ALL_STRINGS(LIST)\ + do {\ + void *destroy_all_temp__ = (LIST);\ + if (destroy_all_temp__) {\ + for (; *(LIST); (LIST)++)\ + free(*(LIST));\ free(destroy_all_temp__);\ }\ } while (0) diff --git a/libcontacts_address_destroy.c b/libcontacts_address_destroy.c index 1a79f21..e8ed733 100644 --- a/libcontacts_address_destroy.c +++ b/libcontacts_address_destroy.c @@ -11,5 +11,5 @@ libcontacts_address_destroy(struct libcontacts_address *this) free(this->address); free(this->postcode); free(this->city); - DESTROY_ALL(this->unrecognised_data, free); + DESTROY_ALL_STRINGS(this->unrecognised_data); } diff --git a/libcontacts_block_destroy.c b/libcontacts_block_destroy.c index 1298aca..2fb53b6 100644 --- a/libcontacts_block_destroy.c +++ b/libcontacts_block_destroy.c @@ -6,5 +6,5 @@ void libcontacts_block_destroy(struct libcontacts_block *this) { free(this->service); - DESTROY_ALL(this->unrecognised_data, free); + DESTROY_ALL_STRINGS(this->unrecognised_data); } diff --git a/libcontacts_chat_destroy.c b/libcontacts_chat_destroy.c index 7e5498a..99bfc7b 100644 --- a/libcontacts_chat_destroy.c +++ b/libcontacts_chat_destroy.c @@ -8,5 +8,5 @@ libcontacts_chat_destroy(struct libcontacts_chat *this) free(this->context); free(this->service); free(this->address); - DESTROY_ALL(this->unrecognised_data, free); + DESTROY_ALL_STRINGS(this->unrecognised_data); } diff --git a/libcontacts_contact_destroy.c b/libcontacts_contact_destroy.c index 70d1335..6ea1825 100644 --- a/libcontacts_contact_destroy.c +++ b/libcontacts_contact_destroy.c @@ -11,17 +11,20 @@ libcontacts_contact_destroy(struct libcontacts_contact *this) free(this->last_name); free(this->full_name); free(this->nickname); - DESTROY_ALL(this->photos, free); - DESTROY_ALL(this->groups, free); + DESTROY_ALL_STRINGS(this->photos); + DESTROY_ALL_STRINGS(this->groups); free(this->notes); - DESTROY_ALL(this->blocks, libcontacts_block_destroy); - DESTROY_ALL(this->organisations, libcontacts_organisation_destroy); - DESTROY_ALL(this->emails, libcontacts_email_destroy); - DESTROY_ALL(this->pgpkeys, libcontacts_pgpkey_destroy); - DESTROY_ALL(this->numbers, libcontacts_number_destroy); - DESTROY_ALL(this->addresses, libcontacts_address_destroy); - DESTROY_ALL(this->sites, libcontacts_site_destroy); - DESTROY_ALL(this->chats, libcontacts_chat_destroy); - libcontacts_birthday_destroy(this->birthday); - DESTROY_ALL(this->unrecognised_data, free); + DESTROY_ALL_OBJECTS(this->blocks, libcontacts_block_destroy); + DESTROY_ALL_OBJECTS(this->organisations, libcontacts_organisation_destroy); + DESTROY_ALL_OBJECTS(this->emails, libcontacts_email_destroy); + DESTROY_ALL_OBJECTS(this->pgpkeys, libcontacts_pgpkey_destroy); + DESTROY_ALL_OBJECTS(this->numbers, libcontacts_number_destroy); + DESTROY_ALL_OBJECTS(this->addresses, libcontacts_address_destroy); + DESTROY_ALL_OBJECTS(this->sites, libcontacts_site_destroy); + DESTROY_ALL_OBJECTS(this->chats, libcontacts_chat_destroy); + if (this->birthday) { + libcontacts_birthday_destroy(this->birthday); + free(this->birthday); + } + DESTROY_ALL_STRINGS(this->unrecognised_data); } diff --git a/libcontacts_email_destroy.c b/libcontacts_email_destroy.c index 329cd62..b8f4ec3 100644 --- a/libcontacts_email_destroy.c +++ b/libcontacts_email_destroy.c @@ -7,5 +7,5 @@ libcontacts_email_destroy(struct libcontacts_email *this) { free(this->context); free(this->address); - DESTROY_ALL(this->unrecognised_data, free); + DESTROY_ALL_STRINGS(this->unrecognised_data); } diff --git a/libcontacts_load_contacts.c b/libcontacts_load_contacts.c index 91f8c56..0bb0b86 100644 --- a/libcontacts_load_contacts.c +++ b/libcontacts_load_contacts.c @@ -17,7 +17,7 @@ libcontacts_load_contacts(struct libcontacts_contact ***contactsp, const struct return -1; for (n = 0; ids[n]; n++); - *contactsp = calloc(n, sizeof(**contactsp)); + *contactsp = calloc(n + 1, sizeof(**contactsp)); if (!*contactsp) goto fail; diff --git a/libcontacts_number_destroy.c b/libcontacts_number_destroy.c index 64b84cd..0acb54a 100644 --- a/libcontacts_number_destroy.c +++ b/libcontacts_number_destroy.c @@ -7,5 +7,5 @@ libcontacts_number_destroy(struct libcontacts_number *this) { free(this->context); free(this->number); - DESTROY_ALL(this->unrecognised_data, free); + DESTROY_ALL_STRINGS(this->unrecognised_data); } diff --git a/libcontacts_organisation_destroy.c b/libcontacts_organisation_destroy.c index 94d4a3a..a65a774 100644 --- a/libcontacts_organisation_destroy.c +++ b/libcontacts_organisation_destroy.c @@ -7,5 +7,5 @@ libcontacts_organisation_destroy(struct libcontacts_organisation *this) { free(this->organisation); free(this->title); - DESTROY_ALL(this->unrecognised_data, free); + DESTROY_ALL_STRINGS(this->unrecognised_data); } diff --git a/libcontacts_parse_contact.c b/libcontacts_parse_contact.c index 2d7ae95..df31559 100644 --- a/libcontacts_parse_contact.c +++ b/libcontacts_parse_contact.c @@ -69,24 +69,35 @@ addstr(char ***listp, const char *new) { size_t i; void *temp, *add; + int error; add = strdup(new); if (!add) return -1; if (!*listp) { *listp = calloc(2, sizeof(char *)); - **listp = add; + if (!*listp) + goto fail_errno; + (*listp)[0] = add; } else { for (i = 0; (*listp)[i]; i++); - temp = realloc(*listp, (i + 2) * sizeof(char *)); - if (!temp) { - free(add); - return -1; + if (i > SIZE_MAX / sizeof(char *) + 2) { + error = ENOMEM; + goto fail; } + temp = realloc(*listp, (i + 2) * sizeof(char *)); + if (!temp) + goto fail_errno; *listp = temp; (*listp)[i + 0] = add; (*listp)[i + 1] = NULL; } return 0; +fail_errno: + error = errno; +fail: + free(add); + errno = error; + return -1; } static int @@ -157,12 +168,18 @@ int libcontacts_parse_contact(char *data, struct libcontacts_contact *contact) { #define TEST(S, L)\ - (!strncmp((S), L, sizeof(L) - 1) &&\ - (S[sizeof(L)] == ' ' || S[sizeof(L)] == '\t')) + (!strncmp((test_tmp = (S)), L, sizeof(L) - 1) &&\ + (test_tmp[sizeof(L) - 1] == ' ' || test_tmp[sizeof(L) - 1] == '\t')) #define ADD(LIST)\ do {\ - for (i = 0; (LIST) && (LIST)[i]; i++);\ + i = 0;\ + if (LIST)\ + for (; (LIST)[i]; i++);\ + if (i > SIZE_MAX / sizeof(*(LIST)) - 2) {\ + errno = ENOMEM;\ + goto fail;\ + }\ temp = realloc((LIST), (i + 2) * sizeof(*(LIST)));\ if (!temp)\ goto fail;\ @@ -172,7 +189,7 @@ libcontacts_parse_contact(char *data, struct libcontacts_contact *contact) goto fail;\ } while (0) - char *p, *q; + char *p, *q, *test_tmp; size_t i; time_t t; void *temp; @@ -235,7 +252,7 @@ libcontacts_parse_contact(char *data, struct libcontacts_contact *contact) if (!(contact->organisations[i]->title = strdup(getstr(p)))) goto fail; } else { - if (addstr(&contact->organisations[i]->unrecognised_data, getstr(p))) + if (addstr(&contact->organisations[i]->unrecognised_data, unindent(p))) goto fail; } break; @@ -252,7 +269,7 @@ libcontacts_parse_contact(char *data, struct libcontacts_contact *contact) if (!(contact->emails[i]->address = strdup(getstr(p)))) goto fail; } else { - if (addstr(&contact->emails[i]->unrecognised_data, getstr(p))) + if (addstr(&contact->emails[i]->unrecognised_data, unindent(p))) goto fail; } break; @@ -269,7 +286,7 @@ libcontacts_parse_contact(char *data, struct libcontacts_contact *contact) if (!(contact->pgpkeys[i]->id = strdup(getstr(p)))) goto fail; } else { - if (addstr(&contact->pgpkeys[i]->unrecognised_data, getstr(p))) + if (addstr(&contact->pgpkeys[i]->unrecognised_data, unindent(p))) goto fail; } break; @@ -290,7 +307,7 @@ libcontacts_parse_contact(char *data, struct libcontacts_contact *contact) } else if (!strcmp(unindent(p), "FAX")) { contact->numbers[i]->is_facsimile = 1; } else { - if (addstr(&contact->numbers[i]->unrecognised_data, getstr(p))) + if (addstr(&contact->numbers[i]->unrecognised_data, unindent(p))) goto fail; } break; @@ -324,11 +341,11 @@ libcontacts_parse_contact(char *data, struct libcontacts_contact *contact) &contact->addresses[i]->longitude)) { contact->addresses[i]->have_coordinates = 1; } else { - if (addstr(&contact->addresses[i]->unrecognised_data, getstr(p))) + if (addstr(&contact->addresses[i]->unrecognised_data, unindent(p))) goto fail; } } else { - if (addstr(&contact->addresses[i]->unrecognised_data, getstr(p))) + if (addstr(&contact->addresses[i]->unrecognised_data, unindent(p))) goto fail; } break; @@ -345,7 +362,7 @@ libcontacts_parse_contact(char *data, struct libcontacts_contact *contact) if (!(contact->sites[i]->address = strdup(getstr(p)))) goto fail; } else { - if (addstr(&contact->sites[i]->unrecognised_data, getstr(p))) + if (addstr(&contact->sites[i]->unrecognised_data, unindent(p))) goto fail; } break; @@ -365,7 +382,7 @@ libcontacts_parse_contact(char *data, struct libcontacts_contact *contact) if (!(contact->chats[i]->address = strdup(getstr(p)))) goto fail; } else { - if (addstr(&contact->chats[i]->unrecognised_data, getstr(p))) + if (addstr(&contact->chats[i]->unrecognised_data, unindent(p))) goto fail; } break; @@ -391,7 +408,7 @@ libcontacts_parse_contact(char *data, struct libcontacts_contact *contact) } else if (TEST(unindent(p), "REMOVE") && !contact->blocks[i]->hard_unblock && (t = gettime(p))) { contact->blocks[i]->hard_unblock = t; } else { - if (addstr(&contact->organisations[i]->unrecognised_data, getstr(p))) + if (addstr(&contact->organisations[i]->unrecognised_data, unindent(p))) goto fail; } break; @@ -412,7 +429,7 @@ libcontacts_parse_contact(char *data, struct libcontacts_contact *contact) } else if (!strcmp(p, "EARLY")) { contact->birthday->before_on_common = 1; } else { - if (addstr(&contact->birthday->unrecognised_data, getstr(p))) + if (addstr(&contact->birthday->unrecognised_data, unindent(p))) goto fail; } break; @@ -430,7 +447,7 @@ libcontacts_parse_contact(char *data, struct libcontacts_contact *contact) contact->gender = LIBCONTACTS_FEMALE; } else { - if (addstr(&contact->unrecognised_data, getstr(p))) + if (addstr(&contact->unrecognised_data, p)) goto fail; } } diff --git a/libcontacts_pgpkey_destroy.c b/libcontacts_pgpkey_destroy.c index aa465ab..9dce05e 100644 --- a/libcontacts_pgpkey_destroy.c +++ b/libcontacts_pgpkey_destroy.c @@ -7,5 +7,5 @@ libcontacts_pgpkey_destroy(struct libcontacts_pgpkey *this) { free(this->context); free(this->id); - DESTROY_ALL(this->unrecognised_data, free); + DESTROY_ALL_STRINGS(this->unrecognised_data); } diff --git a/libcontacts_site_destroy.c b/libcontacts_site_destroy.c index f1cef31..029d9cd 100644 --- a/libcontacts_site_destroy.c +++ b/libcontacts_site_destroy.c @@ -7,5 +7,5 @@ libcontacts_site_destroy(struct libcontacts_site *this) { free(this->context); free(this->address); - DESTROY_ALL(this->unrecognised_data, free); + DESTROY_ALL_STRINGS(this->unrecognised_data); } -- cgit v1.2.3-70-g09d2