diff options
author | cperciva <cperciva@FreeBSD.org> | 2007-07-12 15:00:28 +0000 |
---|---|---|
committer | cperciva <cperciva@FreeBSD.org> | 2007-07-12 15:00:28 +0000 |
commit | a337e4787d450a80f8ce96901d7ee65b5523da73 (patch) | |
tree | c795c52a26211c05dc99bf6a24121801d34fc5e9 | |
parent | ffeb10004251b508a5328f1cd48a6781500dcf56 (diff) | |
download | FreeBSD-src-a337e4787d450a80f8ce96901d7ee65b5523da73.zip FreeBSD-src-a337e4787d450a80f8ce96901d7ee65b5523da73.tar.gz |
Correct multiple security issues in how libarchive handles corrupt
tar archives, including a potentially exploitable buffer overflow.
Approved by: re (kensmith, security blanket)
Reviewed by: kientzle
Security: FreeBSD-SA-07:05.libarchive
-rw-r--r-- | lib/libarchive/Makefile | 2 | ||||
-rw-r--r-- | lib/libarchive/archive_read_support_format_tar.c | 143 |
2 files changed, 104 insertions, 41 deletions
diff --git a/lib/libarchive/Makefile b/lib/libarchive/Makefile index 4203ba7..1952496 100644 --- a/lib/libarchive/Makefile +++ b/lib/libarchive/Makefile @@ -9,7 +9,7 @@ LDADD= -lbz2 -lz # Major: Bumped ONLY when API/ABI breakage happens (see SHLIB_MAJOR) # Minor: Bumped when significant new features are added # Revision: Bumped on any notable change -VERSION= 2.2.3 +VERSION= 2.2.4 ARCHIVE_API_MAJOR!= echo ${VERSION} | sed -e 's/[^0-9]/./g' -e 's/\..*//' ARCHIVE_API_MINOR!= echo ${VERSION} | sed -e 's/[^0-9]/./g' -e 's/[0-9]*\.//' -e 's/\..*//' diff --git a/lib/libarchive/archive_read_support_format_tar.c b/lib/libarchive/archive_read_support_format_tar.c index 1cc1cd6..03088d7 100644 --- a/lib/libarchive/archive_read_support_format_tar.c +++ b/lib/libarchive/archive_read_support_format_tar.c @@ -690,7 +690,13 @@ tar_read_header(struct archive_read *a, struct tar *tar, } } --tar->header_recursion_depth; - return (err); + /* We return warnings or success as-is. Anything else is fatal. */ + if (err == ARCHIVE_WARN || err == ARCHIVE_OK) + return (err); + if (err == ARCHIVE_EOF) + /* EOF when recursively reading a header is bad. */ + archive_set_error(&a->archive, EINVAL, "Damaged tar archive"); + return (ARCHIVE_FATAL); } /* @@ -761,32 +767,55 @@ static int header_Solaris_ACL(struct archive_read *a, struct tar *tar, struct archive_entry *entry, const void *h) { - int err, err2; - char *p; + const struct archive_entry_header_ustar *header; + size_t size; + int err; + char *acl, *p; wchar_t *wp; + /* + * read_body_to_string adds a NUL terminator, but we need a little + * more to make sure that we don't overrun acl_text later. + */ + header = (const struct archive_entry_header_ustar *)h; + size = tar_atol(header->size, sizeof(header->size)); err = read_body_to_string(a, tar, &(tar->acl_text), h); - err2 = tar_read_header(a, tar, entry); - err = err_combine(err, err2); - - /* XXX Ensure p doesn't overrun acl_text */ + if (err != ARCHIVE_OK) + return (err); + err = tar_read_header(a, tar, entry); + if ((err != ARCHIVE_OK) && (err != ARCHIVE_WARN)) + return (err); /* Skip leading octal number. */ /* XXX TODO: Parse the octal number and sanity-check it. */ - p = tar->acl_text.s; - while (*p != '\0') + p = acl = tar->acl_text.s; + while (*p != '\0' && p < acl + size) p++; p++; - wp = (wchar_t *)malloc((strlen(p) + 1) * sizeof(wchar_t)); - if (wp != NULL) { - utf8_decode(wp, p, strlen(p)); - err2 = __archive_entry_acl_parse_w(entry, wp, - ARCHIVE_ENTRY_ACL_TYPE_ACCESS); - err = err_combine(err, err2); - free(wp); + if (p >= acl + size) { + archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC, + "Malformed Solaris ACL attribute"); + return(ARCHIVE_WARN); } + /* Skip leading octal number. */ + size -= (p - acl); + acl = p; + + while (*p != '\0' && p < acl + size) + p++; + + wp = (wchar_t *)malloc((p - acl + 1) * sizeof(wchar_t)); + if (wp == NULL) { + archive_set_error(&a->archive, ENOMEM, + "Can't allocate work buffer for ACL parsing"); + return (ARCHIVE_FATAL); + } + utf8_decode(wp, acl, p - acl); + err = __archive_entry_acl_parse_w(entry, wp, + ARCHIVE_ENTRY_ACL_TYPE_ACCESS); + free(wp); return (err); } @@ -797,15 +826,17 @@ static int header_longlink(struct archive_read *a, struct tar *tar, struct archive_entry *entry, const void *h) { - int err, err2; + int err; err = read_body_to_string(a, tar, &(tar->longlink), h); - err2 = tar_read_header(a, tar, entry); - if (err == ARCHIVE_OK && err2 == ARCHIVE_OK) { - /* Set symlink if symlink already set, else hardlink. */ - archive_entry_set_link(entry, tar->longlink.s); - } - return (err_combine(err, err2)); + if (err != ARCHIVE_OK) + return (err); + err = tar_read_header(a, tar, entry); + if ((err != ARCHIVE_OK) && (err != ARCHIVE_WARN)) + return (err); + /* Set symlink if symlink already set, else hardlink. */ + archive_entry_set_link(entry, tar->longlink.s); + return (ARCHIVE_OK); } /* @@ -815,14 +846,17 @@ static int header_longname(struct archive_read *a, struct tar *tar, struct archive_entry *entry, const void *h) { - int err, err2; + int err; err = read_body_to_string(a, tar, &(tar->longname), h); + if (err != ARCHIVE_OK) + return (err); /* Read and parse "real" header, then override name. */ - err2 = tar_read_header(a, tar, entry); - if (err == ARCHIVE_OK && err2 == ARCHIVE_OK) - archive_entry_set_pathname(entry, tar->longname.s); - return (err_combine(err, err2)); + err = tar_read_header(a, tar, entry); + if ((err != ARCHIVE_OK) && (err != ARCHIVE_WARN)) + return (err); + archive_entry_set_pathname(entry, tar->longname.s); + return (ARCHIVE_OK); } @@ -855,6 +889,11 @@ read_body_to_string(struct archive_read *a, struct tar *tar, (void)tar; /* UNUSED */ header = (const struct archive_entry_header_ustar *)h; size = tar_atol(header->size, sizeof(header->size)); + if ((size > 1048576) || (size < 0)) { + archive_set_error(&a->archive, EINVAL, + "Special header too large"); + return (ARCHIVE_FATAL); + } /* Read the body into the string. */ archive_string_ensure(as, size+1); @@ -862,6 +901,8 @@ read_body_to_string(struct archive_read *a, struct tar *tar, dest = as->s; while (padded_size > 0) { bytes_read = (a->decompressor->read_ahead)(a, &src, padded_size); + if (bytes_read == 0) + return (ARCHIVE_EOF); if (bytes_read < 0) return (ARCHIVE_FATAL); if (bytes_read > padded_size) @@ -1052,11 +1093,13 @@ static int header_pax_global(struct archive_read *a, struct tar *tar, struct archive_entry *entry, const void *h) { - int err, err2; + int err; err = read_body_to_string(a, tar, &(tar->pax_global), h); - err2 = tar_read_header(a, tar, entry); - return (err_combine(err, err2)); + if (err != ARCHIVE_OK) + return (err); + err = tar_read_header(a, tar, entry); + return (err); } static int @@ -1065,10 +1108,14 @@ header_pax_extensions(struct archive_read *a, struct tar *tar, { int err, err2; - read_body_to_string(a, tar, &(tar->pax_header), h); + err = read_body_to_string(a, tar, &(tar->pax_header), h); + if (err != ARCHIVE_OK) + return (err); /* Parse the next header. */ err = tar_read_header(a, tar, entry); + if ((err != ARCHIVE_OK) && (err != ARCHIVE_WARN)) + return (err); /* * TODO: Parse global/default options into 'entry' struct here @@ -1165,8 +1212,11 @@ pax_header(struct archive_read *a, struct tar *tar, l--; break; } - if (*p < '0' || *p > '9') - return (-1); + if (*p < '0' || *p > '9') { + archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC, + "Ignoring malformed pax extended attributes"); + return (ARCHIVE_WARN); + } line_length *= 10; line_length += *p - '0'; if (line_length > 999999) { @@ -1178,8 +1228,19 @@ pax_header(struct archive_read *a, struct tar *tar, l--; } - if (line_length > attr_length) - return (0); + /* + * Parsed length must be no bigger than available data, + * at least 1, and the last character of the line must + * be '\n'. + */ + if (line_length > attr_length + || line_length < 1 + || attr[line_length - 1] != '\n') + { + archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC, + "Ignoring malformed pax extended attribute"); + return (ARCHIVE_WARN); + } /* Ensure pax_entry buffer is big enough. */ if (tar->pax_entry_length <= line_length) { @@ -1962,18 +2023,20 @@ readline(struct archive_read *a, struct tar *tar, const char **start) memcpy(tar->line.s + total_size, t, bytes_read); (a->decompressor->consume)(a, bytes_read); total_size += bytes_read; + /* If we found '\n', clean up and return. */ + if (p != NULL) { + *start = tar->line.s; + return (total_size); + } /* Read some more. */ bytes_read = (a->decompressor->read_ahead)(a, &t, 1); if (bytes_read <= 0) return (ARCHIVE_FATAL); s = t; /* Start of line? */ p = memchr(t, '\n', bytes_read); - /* If we found '\n', finish the line. */ + /* If we found '\n', trim the read. */ if (p != NULL) { bytes_read = 1 + ((const char *)p) - s; - (a->decompressor->consume)(a, bytes_read); - *start = tar->line.s; - return (total_size + bytes_read); } } } |