Skip to content
Permalink
Browse files

Fix a potential crash issue discovered by Alexander Cherepanov:

It seems bsdtar automatically handles stacked compression. This is a
nice feature but it could be problematic when it's completely
unlimited.  Most clearly it's illustrated with quines:

$ curl -sRO http://www.maximumcompression.com/selfgz.gz
$ (ulimit -v 10000000 && bsdtar -tvf selfgz.gz)
bsdtar: Error opening archive: Can't allocate data for gzip decompression

Without ulimit, bsdtar will eat all available memory. This could also
be a problem for other applications using libarchive.
  • Loading branch information...
kientzle committed Jan 10, 2015
1 parent 48b288a commit 6e06b1c89dd0d16f74894eac4cfc1327a06ee4a0
@@ -482,6 +482,7 @@ libarchive_test_SOURCES= \
libarchive/test/test_read_pax_truncated.c \
libarchive/test/test_read_position.c \
libarchive/test/test_read_set_format.c \
libarchive/test/test_read_too_many_filters.c \
libarchive/test/test_read_truncated.c \
libarchive/test/test_read_truncated_filter.c \
libarchive/test/test_sparse_basic.c \
@@ -791,6 +792,7 @@ libarchive_test_EXTRA_DIST=\
libarchive/test/test_read_splitted_rar_ab.uu \
libarchive/test/test_read_splitted_rar_ac.uu \
libarchive/test/test_read_splitted_rar_ad.uu \
libarchive/test/test_read_too_many_filters.gz.uu \
libarchive/test/test_splitted_rar_seek_support_aa.uu \
libarchive/test/test_splitted_rar_seek_support_ab.uu \
libarchive/test/test_splitted_rar_seek_support_ac.uu \
@@ -548,13 +548,13 @@ archive_read_open1(struct archive *_a)
static int
choose_filters(struct archive_read *a)
{
int number_bidders, i, bid, best_bid;
int number_bidders, i, bid, best_bid, n;

This comment has been minimized.

Copy link
@jsonn

jsonn Feb 21, 2016

Contributor

n might be better called number_bids, but more importantly, both number variables should be unsigned?

This comment has been minimized.

Copy link
@kientzle

kientzle Feb 21, 2016

Author Contributor

I've renamed the variable. Unsigned isn't an issue since the numbers involved are always very small.

struct archive_read_filter_bidder *bidder, *best_bidder;
struct archive_read_filter *filter;
ssize_t avail;
int r;

for (;;) {
for (n = 0; n < 25; ++n) {

This comment has been minimized.

Copy link
@jsonn

jsonn Feb 21, 2016

Contributor

Can you introduce a macro for the 25 with an appropriate name? The magic number is otherwise a bit unmotivated and random.

This comment has been minimized.

Copy link
@kientzle

kientzle Feb 21, 2016

Author Contributor

Thanks for pointing that out: Addressed in commit 37649d2

number_bidders = sizeof(a->bidders) / sizeof(a->bidders[0]);

best_bid = 0;
@@ -600,6 +600,9 @@ choose_filters(struct archive_read *a)
return (ARCHIVE_FATAL);
}
}
archive_set_error(&a->archive, ARCHIVE_ERRNO_FILE_FORMAT,
"Input requires too many filters for decoding");
return (ARCHIVE_FATAL);
}

/*
@@ -172,6 +172,7 @@ IF(ENABLE_TEST)
test_read_pax_truncated.c
test_read_position.c
test_read_set_format.c
test_read_too_many_filters.c
test_read_truncated.c
test_read_truncated_filter.c
test_sparse_basic.c
@@ -0,0 +1,45 @@
/*-
* Copyright (c) 2003-2008,2015 Tim Kientzle
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE AUTHOR(S) ``AS IS'' AND ANY EXPRESS OR
* IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
* OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
* IN NO EVENT SHALL THE AUTHOR(S) BE LIABLE FOR ANY DIRECT, INDIRECT,
* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
* NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
* THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
#include "test.h"

DEFINE_TEST(test_read_too_many_filters)
{
const char *name = "test_read_too_many_filters.gz";
struct archive *a;
int r;

assert((a = archive_read_new()) != NULL);
r = archive_read_support_filter_gzip(a);
if (r == ARCHIVE_WARN) {
skipping("gzip reading not fully supported on this platform");
}
assertEqualIntA(a, ARCHIVE_OK, archive_read_support_format_all(a));
extract_reference_file(name);
assertEqualIntA(a, ARCHIVE_FATAL,
archive_read_open_filename(a, name, 200));

assertEqualInt(ARCHIVE_OK, archive_read_close(a));
assertEqualInt(ARCHIVE_OK, archive_read_free(a));
}
@@ -0,0 +1,15 @@
This is a valid gzip file that decompresses to itself, from
http://www.maximumcompression.com/selfgz.gz

This is used in test_read_too_many_filters to try to
crash libarchive by forcing it to spawn an unending
list of gunzip filters.

begin 644 test_read_too_many_filters.gz
M'XL(`````````P`/`/#_'XL(`````````P`/`/#_````__\```#__X)QH5P`
M`!X`X?\```#__P```/__@G&A7```'@#A_P```/__````__\```#__P```/__
M````__\```#__\(FAF`!`!0`Z_\```#__P```/__PB:&8`$`%`#K_\(FAF`!
M`!0`Z_^9(#6-B"@Q,C,T`K/`+```%`#K_P*SP"P``!0`Z_]"B"'$`````/__
>`P!#2DTAT@```$*((<0`````__\#`$-*32'2````
`
end

0 comments on commit 6e06b1c

Please sign in to comment.
You can’t perform that action at this time.