From 0e3b54db6d7ec7c7baf151c812b77042927cf44e Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Sat, 5 Dec 2020 19:55:30 +0100 Subject: CVE-2020-27350: tarfile: integer overflow: Limit tar items to 128 GiB The integer overflow was detected by DonKult who added a check like this: (std::numeric_limits::max() - (2 * sizeof(Block))) Which deals with the code as is, but also still is a fairly big limit, and could become fragile if we change the code. Let's limit our file sizes to 128 GiB, which should be sufficient for everyone. Original comment by DonKult: The code assumes that it can add sizeof(Block)-1 to the size of the item later on, but if we are close to a 64bit overflow this is not possible. Fixing this seems too complex compared to just ensuring there is enough room left given that we will have a lot more problems the moment we will be acting on files that large as if the item is that large, the (valid) tar including it probably doesn't fit in 64bit either. --- apt-inst/contrib/extracttar.cc | 10 ++++++++++ test/integration/test-cve-2020-27350 | 3 +++ test/interactive-helper/createdeb-cve-2020-27350.cc | 4 ++++ 3 files changed, 17 insertions(+) diff --git a/apt-inst/contrib/extracttar.cc b/apt-inst/contrib/extracttar.cc index b24b66261..cbee4d12a 100644 --- a/apt-inst/contrib/extracttar.cc +++ b/apt-inst/contrib/extracttar.cc @@ -61,6 +61,11 @@ struct ExtractTar::TarHeader // us with large streams. static const unsigned long long APT_LONGNAME_LIMIT = 1048576llu; +// A file size limit that we allow extracting. Currently, that's 128 GB. +// We also should leave some wiggle room for code adding files to it, and +// possibly conversion for signed, so this should not be larger than like 2**62. +static const unsigned long long APT_FILESIZE_LIMIT = 1llu << 37; + // ExtractTar::ExtractTar - Constructor /*{{{*/ // --------------------------------------------------------------------- /* */ @@ -175,6 +180,11 @@ bool ExtractTar::Go(pkgDirStream &Stream) StrToNum(Tar->Minor,Itm.Minor,sizeof(Tar->Minor),8) == false) return _error->Error(_("Corrupted archive")); + // Security check. Prevents overflows below the code when rounding up in skip/copy code, + // and provides modest protection against decompression bombs. + if (Itm.Size > APT_FILESIZE_LIMIT) + return _error->Error("Tar member too large: %llu > %llu bytes", Itm.Size, APT_FILESIZE_LIMIT); + // Grab the filename and link target: use last long name if one was // set, otherwise use the header value as-is, but remember that it may // fill the entire 100-byte block and needs to be zero-terminated. diff --git a/test/integration/test-cve-2020-27350 b/test/integration/test-cve-2020-27350 index f4bb79bcb..a32bf95e5 100755 --- a/test/integration/test-cve-2020-27350 +++ b/test/integration/test-cve-2020-27350 @@ -20,3 +20,6 @@ testequal "E: Long name to large: 67108865 bytes > 1048576 bytes" runapt ${APTTE ${APTTESTHELPERSBINDIR}/createdeb-cve-2020-27350 long-control long-control.deb testequal "E: Control file too large: 67108865 > 67108864 bytes" runapt ${APTTESTHELPERSBINDIR}/extract-control long-control.deb control + +${APTTESTHELPERSBINDIR}/createdeb-cve-2020-27350 too-long-control too-long-control.deb +testequal "E: Tar member too large: $((128 * 1024 * 1024 * 1024 + 1)) > $((128 * 1024 * 1024 * 1024)) bytes" runapt ${APTTESTHELPERSBINDIR}/extract-control too-long-control.deb control diff --git a/test/interactive-helper/createdeb-cve-2020-27350.cc b/test/interactive-helper/createdeb-cve-2020-27350.cc index af049d4e8..8b9619469 100644 --- a/test/interactive-helper/createdeb-cve-2020-27350.cc +++ b/test/interactive-helper/createdeb-cve-2020-27350.cc @@ -307,6 +307,10 @@ int main(int argc, char *argv[]) { createdeb_bigtarfilelength(fd, '0'); } + else if (strcmp(mode, "too-long-control") == 0) + { + createdeb_bigtarfilelength(fd, '0', 128llu * 1024 * 1024 * 1024 + 1); + } else if (strcmp(mode, "test") == 0) { createdeb_test(fd); -- cgit v1.2.3