Date: Thu, 14 Jul 2005 21:25:53 +0200 (CEST) From: Dan Lukes <dan@obluda.cz> To: FreeBSD-gnats-submit@FreeBSD.org Subject: bin/83476: [ PATCH ] Too many unhandled malloc errors within libarchive Message-ID: <200507141925.j6EJPr9a035236@kulesh.obluda.cz> Resent-Message-ID: <200507141930.j6EJUAJH055648@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 83476 >Category: bin >Synopsis: [ PATCH ] Too many unhandled malloc errors within libarchive >Confidential: no >Severity: serious >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Thu Jul 14 19:30:09 GMT 2005 >Closed-Date: >Last-Modified: >Originator: Dan Lukes >Release: FreeBSD 5.4-STABLE i386 >Organization: Obludarium >Environment: System: FreeBSD 5.4-STABLE #8: Sat Jul 9 16:31:08 CEST 2005 i386 lib/libarchive/archive_read.c,v 1.12.2.2 2005/05/16 04:32:16 lib/libarchive/archive_read_support_format_cpio.c,v 1.11.2.2 2005/05/16 04:32:16 lib/libarchive/archive_write_set_format_pax.c,v 1.17.2.4 2005/05/16 04:37:59 lib/libarchive/archive_read_open_file.c,v 1.6.2.2 2005/05/16 02:47:34 lib/libarchive/archive_entry.c,v 1.23.2.3 2005/05/16 04:37:59 lib/libarchive/archive_read_open_fd.c,v 1.3 2004/06/27 23:36:39 lib/libarchive/archive_read_support_format_tar.c,v 1.26.2.6 2005/05/16 04:32:16 lib/libarchive/archive_read_support_format_zip.c,v 1.4.2.2 2005/05/16 04:32:16 lib/libarchive/archive_read_support_format_iso9660.c,v 1.7.2.2 2005/05/16 04:32:16 >Description: There are many places within libarchive where the return from malloc isn't checked for errors Dereference of NULL (and abend) and/or memory leaks may occur. The patches within the function follow error-handling style already used the within function. archive_set_error() is called when apropriate. Unfortunatelly, there are few function using malloc which has no method to report error to caller. Then I use __archive_errx(1, ...) >How-To-Repeat: >Fix: --- patch begins here --- --- lib/libarchive/archive_read.c.ORIG Tue May 17 16:36:08 2005 +++ lib/libarchive/archive_read.c Thu Jul 14 18:09:48 2005 @@ -55,19 +55,22 @@ archive_read_new(void) { struct archive *a; - char *nulls; - a = malloc(sizeof(*a)); - memset(a, 0, sizeof(*a)); + if ((a = calloc(1, sizeof(*a))) == NULL) { + archive_set_error(a, ENOMEM, "Can't allocate archive object"); + return(NULL); + } a->user_uid = geteuid(); a->magic = ARCHIVE_READ_MAGIC; a->bytes_per_block = ARCHIVE_DEFAULT_BYTES_PER_BLOCK; a->null_length = 1024; - nulls = malloc(a->null_length); - memset(nulls, 0, a->null_length); - a->nulls = nulls; + if ((a->nulls = calloc(1, a->null_length)) == NULL) { + archive_set_error(a, ENOMEM, "Can't allocate archive object 'nulls' element"); + free(a); + return(NULL); + } a->state = ARCHIVE_STATE_NEW; a->entry = archive_entry_new(); --- lib/libarchive/archive_read_support_format_cpio.c.ORIG Tue May 17 16:36:08 2005 +++ lib/libarchive/archive_read_support_format_cpio.c Thu Jul 14 18:42:39 2005 @@ -133,8 +133,10 @@ struct cpio *cpio; int r; - cpio = malloc(sizeof(*cpio)); - memset(cpio, 0, sizeof(*cpio)); + if ((cpio = calloc(1, sizeof(*cpio))) == NULL) { + archive_set_error(a, ENOMEM, "Can't allocate cpio data"); + return (ARCHIVE_FATAL); + } cpio->magic = CPIO_MAGIC; r = __archive_read_register_format(a, @@ -576,6 +578,8 @@ } le = malloc(sizeof(struct links_entry)); + if (le == NULL) + __archive_errx(1, "Out of memory adding file to list"); if (cpio->links_head != NULL) cpio->links_head->previous = le; le->next = cpio->links_head; @@ -585,4 +589,6 @@ le->ino = st->st_ino; le->links = st->st_nlink - 1; le->name = strdup(archive_entry_pathname(entry)); + if (le->name == NULL) + __archive_errx(1, "Out of memory adding file to list"); } --- lib/libarchive/archive_write_set_format_pax.c.ORIG Tue May 17 16:36:09 2005 +++ lib/libarchive/archive_write_set_format_pax.c Thu Jul 14 18:41:54 2005 @@ -93,12 +93,11 @@ if (a->format_finish != NULL) (a->format_finish)(a); - pax = malloc(sizeof(*pax)); + pax = calloc(1, sizeof(*pax)); if (pax == NULL) { archive_set_error(a, ENOMEM, "Can't allocate pax data"); return (ARCHIVE_FATAL); } - memset(pax, 0, sizeof(*pax)); a->format_data = pax; a->pad_uncompressed = 1; @@ -209,6 +208,9 @@ } utf8_value = malloc(utf8len + 1); + if (utf8_value == NULL) { + __archive_errx(1, "Not enough memory for attributes"); + } for (wp = wval, p = utf8_value; *wp != L'\0'; ) { wc = *wp++; if (wc <= 0x7f) { --- lib/libarchive/archive_read_open_file.c.ORIG Tue May 17 16:36:08 2005 +++ lib/libarchive/archive_read_open_file.c Thu Jul 14 20:24:20 2005 @@ -83,6 +83,10 @@ struct stat st; mine->buffer = malloc(mine->block_size); + if (mine->buffer == NULL) { + archive_set_error(a, ENOMEM, "No memory"); + return (ARCHIVE_FATAL); + } if (mine->filename[0] != '\0') mine->fd = open(mine->filename, O_RDONLY); else --- lib/libarchive/archive_entry.c.ORIG Tue May 17 16:36:08 2005 +++ lib/libarchive/archive_entry.c Thu Jul 14 20:26:24 2005 @@ -41,6 +41,7 @@ #include "archive.h" #include "archive_entry.h" +#include "archive_private.h" #undef max #define max(a, b) ((a)>(b)?(a):(b)) @@ -163,12 +164,16 @@ if (src->aes_mbs != NULL) { dest->aes_mbs_alloc = strdup(src->aes_mbs); dest->aes_mbs = dest->aes_mbs_alloc; + if (dest->aes_mbs == NULL) + __archive_errx(1, "No memory for aes_copy()"); } if (src->aes_wcs != NULL) { dest->aes_wcs_alloc = malloc((wcslen(src->aes_wcs) + 1) * sizeof(wchar_t)); dest->aes_wcs = dest->aes_wcs_alloc; + if (dest->aes_wcs == NULL) + __archive_errx(1, "No memory for aes_copy()"); wcscpy(dest->aes_wcs_alloc, src->aes_wcs); } } @@ -186,6 +191,8 @@ int mbs_length = wcslen(aes->aes_wcs) * 3 + 64; aes->aes_mbs_alloc = malloc(mbs_length); aes->aes_mbs = aes->aes_mbs_alloc; + if (aes->aes_mbs == NULL) + __archive_errx(1, "No memory for aes_get_mbs()"); wcstombs(aes->aes_mbs_alloc, aes->aes_wcs, mbs_length - 1); aes->aes_mbs_alloc[mbs_length - 1] = 0; } @@ -204,6 +211,8 @@ aes->aes_wcs_alloc = malloc((wcs_length + 1) * sizeof(wchar_t)); aes->aes_wcs = aes->aes_wcs_alloc; + if (aes->aes_wcs == NULL) + __archive_errx(1, "No memory for aes_get_wcs()"); mbstowcs(aes->aes_wcs_alloc, aes->aes_mbs, wcs_length); aes->aes_wcs_alloc[wcs_length] = 0; } @@ -237,6 +246,8 @@ aes->aes_wcs_alloc = NULL; } aes->aes_mbs_alloc = malloc((strlen(mbs) + 1) * sizeof(char)); + if (aes->aes_mbs_alloc == NULL) + __archive_errx(1, "No memory for aes_copy_mbs()"); strcpy(aes->aes_mbs_alloc, mbs); aes->aes_mbs = aes->aes_mbs_alloc; aes->aes_wcs = NULL; @@ -272,6 +283,8 @@ } aes->aes_mbs = NULL; aes->aes_wcs_alloc = malloc((wcslen(wcs) + 1) * sizeof(wchar_t)); + if (aes->aes_wcs_alloc == NULL) + __archive_errx(1, "No memory for aes_copy_wcs()"); wcscpy(aes->aes_wcs_alloc, wcs); aes->aes_wcs = aes->aes_wcs_alloc; } @@ -296,10 +309,9 @@ struct archive_entry *entry2; /* Allocate new structure and copy over all of the fields. */ - entry2 = malloc(sizeof(*entry2)); + entry2 = calloc(1, sizeof(*entry2)); if(entry2 == NULL) return (NULL); - memset(entry2, 0, sizeof(*entry2)); entry2->ae_stat = entry->ae_stat; entry2->ae_fflags_set = entry->ae_fflags_set; entry2->ae_fflags_clear = entry->ae_fflags_clear; @@ -325,13 +337,7 @@ struct archive_entry * archive_entry_new(void) { - struct archive_entry *entry; - - entry = malloc(sizeof(*entry)); - if(entry == NULL) - return (NULL); - memset(entry, 0, sizeof(*entry)); - return (entry); + return ((struct archive_entry *)calloc(1, sizeof(struct archive_entry))); } /* @@ -791,8 +797,9 @@ } /* Add a new entry to the list. */ - ap = malloc(sizeof(*ap)); - memset(ap, 0, sizeof(*ap)); + ap = calloc(1, sizeof(*ap)); + if (ap == NULL) + return(NULL); ap->next = entry->acl_head; entry->acl_head = ap; ap->type = type; @@ -972,6 +979,8 @@ /* Now, allocate the string and actually populate it. */ wp = entry->acl_text_w = malloc(length * sizeof(wchar_t)); + if (wp == NULL) + __archive_errx(1, "No memory to generate the text version of the ACL"); count = 0; if ((flags & ARCHIVE_ENTRY_ACL_TYPE_ACCESS) != 0) { append_entry_w(&wp, NULL, ARCHIVE_ENTRY_ACL_USER_OBJ, NULL, @@ -1225,6 +1234,8 @@ namebuff_length = name_end - name_start + 256; namebuff = malloc(namebuff_length * sizeof(wchar_t)); + if (namebuff == NULL) + goto fail; } wmemcpy(namebuff, name_start, name_end - name_start); namebuff[name_end - name_start] = L'\0'; --- lib/libarchive/archive_read_open_fd.c.ORIG Sun Aug 8 21:03:13 2004 +++ lib/libarchive/archive_read_open_fd.c Thu Jul 14 20:18:20 2005 @@ -58,6 +58,11 @@ } mine->block_size = block_size; mine->buffer = malloc(mine->block_size); + if (mine->buffer == NULL) { + archive_set_error(a, ENOMEM, "No memory"); + free(mine); + return (ARCHIVE_FATAL); + } mine->fd = fd; return (archive_read_open(a, mine, file_open, file_read, file_close)); } @@ -94,6 +99,7 @@ struct read_fd_data *mine = client_data; (void)a; /* UNUSED */ + free(mine->buffer); free(mine); return (ARCHIVE_OK); } --- lib/libarchive/archive_read_support_format_tar.c.ORIG Tue May 17 16:36:08 2005 +++ lib/libarchive/archive_read_support_format_tar.c Thu Jul 14 20:40:40 2005 @@ -209,8 +209,10 @@ struct tar *tar; int r; - tar = malloc(sizeof(*tar)); - memset(tar, 0, sizeof(*tar)); + if ((tar = calloc(1, sizeof(*tar))) == NULL) { + archive_set_error(a, ENOMEM, "Can't allocate tar data"); + return(ARCHIVE_FATAL); + } r = __archive_read_register_format(a, tar, archive_read_format_tar_bid, @@ -1040,9 +1042,13 @@ while (tar->pax_entry_length <= line_length + 1) tar->pax_entry_length *= 2; - /* XXX Error handling here */ - tar->pax_entry = realloc(tar->pax_entry, + tar->pax_entry = reallocf(tar->pax_entry, tar->pax_entry_length * sizeof(wchar_t)); + if (tar->pax_entry == NULL) { + archive_set_error(a, ENOMEM, + "No memory"); + return(ARCHIVE_FATAL); + } } /* Decode UTF-8 to wchar_t, null-terminate result. */ @@ -1362,8 +1368,9 @@ last = last->next; while (length > 0 && sparse->offset[0] != 0) { - p = malloc(sizeof(*p)); - memset(p, 0, sizeof(*p)); + p = calloc(1, sizeof(*p)); + if (p == NULL) + __archive_errx(1, "Out of memory"); if (last != NULL) last->next = p; else --- lib/libarchive/archive_read_support_format_zip.c.ORIG Tue May 17 16:36:09 2005 +++ lib/libarchive/archive_read_support_format_zip.c Thu Jul 14 20:32:57 2005 @@ -137,8 +137,10 @@ struct zip *zip; int r; - zip = malloc(sizeof(*zip)); - memset(zip, 0, sizeof(*zip)); + if ((zip = calloc(1, sizeof(*zip))) == NULL) { + archive_set_error(a, ENOMEM, "Can't allocate zip data"); + return(ARCHIVE_FATAL); + } r = __archive_read_register_format(a, zip, --- lib/libarchive/archive_read_support_format_iso9660.c.ORIG Tue May 17 16:36:08 2005 +++ lib/libarchive/archive_read_support_format_iso9660.c Thu Jul 14 20:52:32 2005 @@ -120,7 +120,7 @@ unsigned char name_len[1]; char name[1]; }; - +#define ISO9660_DIRECTORY_RECORD_SIZE 33 /* * Our private data. @@ -202,8 +202,10 @@ struct iso9660 *iso9660; int r; - iso9660 = malloc(sizeof(*iso9660)); - memset(iso9660, 0, sizeof(*iso9660)); + if ((iso9660 = calloc(1, sizeof(*iso9660))) == NULL) { + archive_set_error(a, ENOMEM, "Can't allocate iso9660 data"); + return(ARCHIVE_FATAL); + } iso9660->magic = ISO9660_MAGIC; iso9660->bid = -1; /* We haven't yet bid. */ @@ -464,8 +466,9 @@ /* TODO: Sanity check that name_len doesn't exceed length, etc. */ /* Create a new file entry and copy data from the ISO dir record. */ - file = malloc(sizeof(*file)); - memset(file, 0, sizeof(*file)); + if ((file = calloc(1, sizeof(*file))) == NULL) { + return(NULL); + } file->parent = parent; if (parent != NULL) parent->refcount++; @@ -475,6 +478,10 @@ file->mtime = isodate7(isodirrec->date); file->ctime = file->atime = file->mtime; file->name = malloc(isodirrec->name_len[0] + 1); + if (file->name == NULL) { + free(file); + return(NULL); + } memcpy(file->name, isodirrec->name, isodirrec->name_len[0]); file->name[(int)isodirrec->name_len[0]] = '\0'; if (isodirrec->flags[0] & 0x02) @@ -531,6 +538,8 @@ if (new_size < 1024) new_size = 1024; new_pending_files = malloc(new_size * sizeof(new_pending_files[0])); + if (new_pending_files == NULL) + __archive_errx(1, "Out of memory"); memcpy(new_pending_files, iso9660->pending_files, iso9660->pending_files_allocated * sizeof(new_pending_files[0])); if (iso9660->pending_files != NULL) --- patch ends here --- >Release-Note: >Audit-Trail: >Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200507141925.j6EJPr9a035236>