bin/83476: [ PATCH ] Too many unhandled malloc errors within
libarchive
Dan Lukes
dan at obluda.cz
Thu Jul 14 19:30:12 GMT 2005
>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:
More information about the freebsd-bugs
mailing list