From d10c68d628fe5342d400a999a6d10c5c7c0cef41 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Mon, 19 Oct 2020 13:22:33 +0200 Subject: CVE-2020-27350: arfile: Integer overflow in parsing GHSL-2020-169: This first hunk adds a check that we have more files left to read in the file than the size of the member, ensuring that (a) the number is not negative, which caused the crash here and (b) ensures that we similarly avoid other issues with trying to read too much data. GHSL-2020-168: Long file names are encoded by a special marker in the filename and then the real filename is part of what is normally the data. We did not check that the length of the file name is within the length of the member, which means that we got a overflow later when subtracting the length from the member size to get the remaining member size. The file createdeb-lp1899193.cc was provided by GitHub Security Lab and reformatted using apt coding style for inclusion in the test case, both of these issues have an automated test case in test/integration/test-ubuntu-bug-1899193-security-issues. LP: #1899193 --- apt-pkg/contrib/arfile.cc | 14 +- test/integration/test-cve-2020-27350 | 13 ++ test/interactive-helper/CMakeLists.txt | 2 + .../interactive-helper/createdeb-cve-2020-27350.cc | 235 +++++++++++++++++++++ 4 files changed, 263 insertions(+), 1 deletion(-) create mode 100755 test/integration/test-cve-2020-27350 create mode 100644 test/interactive-helper/createdeb-cve-2020-27350.cc diff --git a/apt-pkg/contrib/arfile.cc b/apt-pkg/contrib/arfile.cc index 5cb43c690..6d4a1f158 100644 --- a/apt-pkg/contrib/arfile.cc +++ b/apt-pkg/contrib/arfile.cc @@ -94,7 +94,12 @@ bool ARArchive::LoadHeaders() delete Memb; return _error->Error(_("Invalid archive member header")); } - + + if (Left < 0 || Memb->Size > static_cast(Left)) + { + delete Memb; + return _error->Error(_("Invalid archive member header")); + } // Check for an extra long name string if (memcmp(Head.Name,"#1/",3) == 0) { @@ -106,6 +111,13 @@ bool ARArchive::LoadHeaders() delete Memb; return _error->Error(_("Invalid archive member header")); } + + if (Len > Memb->Size) + { + delete Memb; + return _error->Error(_("Invalid archive member header")); + } + if (File.Read(S,Len) == false) { delete Memb; diff --git a/test/integration/test-cve-2020-27350 b/test/integration/test-cve-2020-27350 new file mode 100755 index 000000000..6ee867bb3 --- /dev/null +++ b/test/integration/test-cve-2020-27350 @@ -0,0 +1,13 @@ +#!/bin/sh +set -e + +TESTDIR="$(readlink -f "$(dirname "$0")")" +. "$TESTDIR/framework" +setupenvironment +configarchitecture "amd64" + +${APTTESTHELPERSBINDIR}/createdeb-cve-2020-27350 crash crash.deb +testequal "E: Invalid archive member header" runapt ${APTTESTHELPERSBINDIR}/testdeb ./crash.deb + +${APTTESTHELPERSBINDIR}/createdeb-cve-2020-27350 loop loop.deb +testequal "E: Invalid archive member header" runapt ${APTTESTHELPERSBINDIR}/testdeb ./loop.deb diff --git a/test/interactive-helper/CMakeLists.txt b/test/interactive-helper/CMakeLists.txt index f4238665d..565474afd 100644 --- a/test/interactive-helper/CMakeLists.txt +++ b/test/interactive-helper/CMakeLists.txt @@ -10,6 +10,8 @@ add_executable(aptdropprivs aptdropprivs.cc) target_link_libraries(aptdropprivs apt-pkg) add_executable(test_fileutl test_fileutl.cc) target_link_libraries(test_fileutl apt-pkg) +add_executable(createdeb-cve-2020-27350 createdeb-cve-2020-27350.cc) + add_library(noprofile SHARED libnoprofile.c) target_link_libraries(noprofile ${CMAKE_DL_LIBS}) diff --git a/test/interactive-helper/createdeb-cve-2020-27350.cc b/test/interactive-helper/createdeb-cve-2020-27350.cc new file mode 100644 index 000000000..9103c2137 --- /dev/null +++ b/test/interactive-helper/createdeb-cve-2020-27350.cc @@ -0,0 +1,235 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include + +struct Header +{ + char Name[16]; + char MTime[12]; + char UID[6]; + char GID[6]; + char Mode[8]; + char Size[10]; + char Magic[2]; +}; + +struct TarHeader +{ + char Name[100]; + char Mode[8]; + char UserID[8]; + char GroupID[8]; + char Size[12]; + char MTime[12]; + char Checksum[8]; + char LinkFlag; + char LinkName[100]; + char MagicNumber[8]; + char UserName[32]; + char GroupName[32]; + char Major[8]; + char Minor[8]; +}; + +// Call `write` and check the result. +static void write_chk(int fd, const void *buf, size_t count) +{ + const ssize_t wr = write(fd, buf, count); + if (wr < 0) + { + const int err = errno; + fprintf(stderr, "Write failed: %s\n", strerror(err)); + exit(EXIT_FAILURE); + } + if ((size_t)wr != count) + { + fprintf(stderr, "Incomplete write.\n"); + exit(EXIT_FAILURE); + } +} + +// Triggers a negative integer overflow at https://git.launchpad.net/ubuntu/+source/apt/tree/apt-pkg/contrib/arfile.cc?h=applied/ubuntu/focal-updates&id=4c264e60b524855b211751e1632ba48526f6b44d#n116: +// +// Memb->Size -= Len; +// +// Due to the integer overflow, the value of Memb->Size is 0xFFFFFFFFFFFFFFFF. +// This leads to an out-of-memory error at https://git.launchpad.net/ubuntu/+source/python-apt/tree/python/arfile.cc?h=applied/ubuntu/focal-updates&id=0f7cc93acdb51d943114f1cd79002288c4ca4d24#n602: +// +// char* value = new char[member->Size]; +// +// The out-of-memory error causes aptd to crash. +static void createdeb_crash(const int fd) +{ + // Magic number + static const char *magic = "!\n"; + write_chk(fd, magic, strlen(magic)); + + struct Header h; + memset(&h, 0, sizeof(h)); + + memcpy(h.Name, "control.tar ", sizeof(h.Name)); + write_chk(fd, &h, sizeof(h)); + + memset(&h, 0, sizeof(h)); + memcpy(h.Name, "data.tar ", sizeof(h.Name)); + write_chk(fd, &h, sizeof(h)); + + memset(&h, 0, sizeof(h)); + memcpy(h.Name, "#1/13 ", sizeof(h.Name)); + strcpy(h.Size, "12"); + write_chk(fd, &h, sizeof(h)); + write_chk(fd, "debian-binary", 13); +} + +// Triggers an infinite loop in `ARArchive::LoadHeaders()`. +// The bug is due to the use of `strtoul` at https://git.launchpad.net/ubuntu/+source/apt/tree/apt-pkg/contrib/strutl.cc?h=applied/ubuntu/focal-updates&id=4c264e60b524855b211751e1632ba48526f6b44d#n1169: +// +// Res = strtoul(S,&End,Base); +// +// The problem is that `strtoul` accepts negative numbers. We exploit that here by setting the size string to "-60". +static void createdeb_loop(const int fd) +{ + // Magic number + static const char *magic = "!\n"; + write_chk(fd, magic, strlen(magic)); + + struct Header h; + memset(&h, 0, sizeof(h)); + + memcpy(h.Name, "#1/20 ", sizeof(h.Name)); + strcpy(h.Size, "-60"); + write_chk(fd, &h, sizeof(h)); + + char buf[20]; + memset(buf, 0, sizeof(buf)); + write_chk(fd, buf, sizeof(buf)); +} + +// Leaks a file descriptor in `debfile_new()`: +// +// https://git.launchpad.net/python-apt/tree/python/arfile.cc?h=2.0.0#n588 +// +// If the .deb file is invalid then the function returns without deleting +// `self`, which means that the file descriptor isn't closed. +static void createdeb_leakfd(const int fd) +{ + // Magic number + static const char *magic = "!\n"; + write_chk(fd, magic, strlen(magic)); + + struct Header h; + memset(&h, 0, sizeof(h)); + + memset(&h, 0, sizeof(h)); + memcpy(h.Name, "data.tar ", sizeof(h.Name)); + write_chk(fd, &h, sizeof(h)); +} + +static void set_checksum(unsigned char block[512]) +{ + struct TarHeader *tar = (struct TarHeader *)&block[0]; + memset(tar->Checksum, ' ', sizeof(tar->Checksum)); + uint32_t sum = 0; + for (int i = 0; i < 512; i++) + { + sum += block[i]; + } + snprintf(tar->Checksum, sizeof(tar->Checksum), "%o", sum); +} + +// This does not trigger a vulnerability. It is just a basis for exploration. +static void createdeb_test(const int fd) +{ + // Magic number + static const char *magic = "!\n"; + write_chk(fd, magic, strlen(magic)); + + struct Header h; + + memset(&h, 0, sizeof(h)); + memcpy(h.Name, "data.tar ", sizeof(h.Name)); + write_chk(fd, &h, sizeof(h)); + + memset(&h, 0, sizeof(h)); + memcpy(h.Name, "debian-binary ", sizeof(h.Name)); + strcpy(h.Size, "4"); + write_chk(fd, &h, sizeof(h)); + static const char *debian_binary = "2.0\n"; + write_chk(fd, debian_binary, strlen(debian_binary)); + + static const char *control = + "Architecture: all\n" + "Package: kevsh\n\n"; + memset(&h, 0, sizeof(h)); + memcpy(h.Name, "control.tar ", sizeof(h.Name)); + snprintf(h.Size, sizeof(h.Size), "%ld", (size_t)512 + 512); + write_chk(fd, &h, sizeof(h)); + + unsigned char block[512]; + memset(block, 0, sizeof(block)); + struct TarHeader *tar = (struct TarHeader *)&block[0]; + strcpy(tar->Name, "control"); + strcpy(tar->Mode, "644"); + snprintf(tar->Size, sizeof(tar->Size), "%lo", strlen(control)); + set_checksum(block); + write_chk(fd, block, sizeof(block)); + + memset(block, 0, sizeof(block)); + strcpy((char *)block, control); + write_chk(fd, block, sizeof(block)); +} + +int main(int argc, char *argv[]) +{ + if (argc != 3) + { + const char *progname = argc > 0 ? argv[0] : "a.out"; + fprintf( + stderr, + "usage: %s \n" + "modes: loop, segv, leakfd\n", + progname); + return EXIT_FAILURE; + } + + const char *mode = argv[1]; + const char *filename = argv[2]; + + const int fd = open(filename, O_CREAT | O_WRONLY | O_TRUNC, 00644); + if (fd < 0) + { + const int err = errno; + fprintf(stderr, "Could not open %s: %s\n", filename, strerror(err)); + return EXIT_FAILURE; + } + + if (strcmp(mode, "crash") == 0) + { + createdeb_crash(fd); + } + else if (strcmp(mode, "loop") == 0) + { + createdeb_loop(fd); + } + else if (strcmp(mode, "leakfd") == 0) + { + createdeb_leakfd(fd); + } + else if (strcmp(mode, "test") == 0) + { + createdeb_test(fd); + } + else + { + fprintf(stderr, "Mode not recognized: %s\n", mode); + } + + close(fd); + return EXIT_SUCCESS; +} -- cgit v1.2.3 From 822db13d68658a1a20df2d19c688c18faa331616 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Fri, 4 Dec 2020 12:37:19 +0100 Subject: tarfile: OOM hardening: Limit size of long names/links to 1 MiB Tarballs have long names and long link targets structured by a special tar header with a GNU extension followed by the actual content (padded to 512 bytes). Essentially, think of a name as a special kind of file. The limit of a file size in a header is 12 bytes, aka 10**12 or 1 TB. While this works OK-ish for file content that we stream to extractors, we need to copy file names into memory, and this opens us up to an OOM DoS attack. Limit the file name size to 1 MiB, as libarchive does, to make things safer. --- apt-pkg/contrib/extracttar.cc | 11 ++- test/integration/test-cve-2020-27350 | 6 ++ .../interactive-helper/createdeb-cve-2020-27350.cc | 84 +++++++++++++++++++++- 3 files changed, 99 insertions(+), 2 deletions(-) diff --git a/apt-pkg/contrib/extracttar.cc b/apt-pkg/contrib/extracttar.cc index 1616c9f12..f653e227e 100644 --- a/apt-pkg/contrib/extracttar.cc +++ b/apt-pkg/contrib/extracttar.cc @@ -55,7 +55,12 @@ struct ExtractTar::TarHeader char Major[8]; char Minor[8]; }; - + +// We need to read long names (names and link targets) into memory, so let's +// have a limit (shamelessly stolen from libarchive) to avoid people OOMing +// us with large streams. +static const unsigned long long APT_LONGNAME_LIMIT = 1048576llu; + // ExtractTar::ExtractTar - Constructor /*{{{*/ // --------------------------------------------------------------------- /* */ @@ -218,6 +223,8 @@ bool ExtractTar::Go(pkgDirStream &Stream) { unsigned long long Length = Itm.Size; unsigned char Block[512]; + if (Length > APT_LONGNAME_LIMIT) + return _error->Error("Long name to large: %llu bytes > %llu bytes", Length, APT_LONGNAME_LIMIT); while (Length > 0) { if (InFd.Read(Block,sizeof(Block),true) == false) @@ -237,6 +244,8 @@ bool ExtractTar::Go(pkgDirStream &Stream) { unsigned long long Length = Itm.Size; unsigned char Block[512]; + if (Length > APT_LONGNAME_LIMIT) + return _error->Error("Long name to large: %llu bytes > %llu bytes", Length, APT_LONGNAME_LIMIT); while (Length > 0) { if (InFd.Read(Block,sizeof(Block),true) == false) diff --git a/test/integration/test-cve-2020-27350 b/test/integration/test-cve-2020-27350 index 6ee867bb3..336dc5b7e 100755 --- a/test/integration/test-cve-2020-27350 +++ b/test/integration/test-cve-2020-27350 @@ -11,3 +11,9 @@ testequal "E: Invalid archive member header" runapt ${APTTESTHELPERSBINDIR}/test ${APTTESTHELPERSBINDIR}/createdeb-cve-2020-27350 loop loop.deb testequal "E: Invalid archive member header" runapt ${APTTESTHELPERSBINDIR}/testdeb ./loop.deb + +${APTTESTHELPERSBINDIR}/createdeb-cve-2020-27350 long-name long-name.deb +testequal "E: Long name to large: 67108865 bytes > 1048576 bytes" runapt ${APTTESTHELPERSBINDIR}/extract-control long-name.deb control + +${APTTESTHELPERSBINDIR}/createdeb-cve-2020-27350 long-link long-link.deb +testequal "E: Long name to large: 67108865 bytes > 1048576 bytes" runapt ${APTTESTHELPERSBINDIR}/extract-control long-link.deb control diff --git a/test/interactive-helper/createdeb-cve-2020-27350.cc b/test/interactive-helper/createdeb-cve-2020-27350.cc index 9103c2137..7c58eb9df 100644 --- a/test/interactive-helper/createdeb-cve-2020-27350.cc +++ b/test/interactive-helper/createdeb-cve-2020-27350.cc @@ -142,8 +142,82 @@ static void set_checksum(unsigned char block[512]) } snprintf(tar->Checksum, sizeof(tar->Checksum), "%o", sum); } +static void base256_encode(char *Str, unsigned long long Num, unsigned int Len) +{ + Str += Len; + while (Len-- > 0) { + *--Str = static_cast(Num & 0xff); + Num >>= 8; + } + + *Str |= 0x80; // mark as base256 +} + +// Create a deb with a control.tar that contains a too large file or link name (GNU extension) +static void createdeb_bigtarfilelength(const int fd, int flag, unsigned long long size = 64llu * 1024 * 1024 + 1) +{ + // Magic number + static const char *magic = "!\n"; + write_chk(fd, magic, strlen(magic)); + + struct Header h; + memset(&h, ' ', sizeof(h)); + memcpy(h.Name, "debian-binary/ ", sizeof(h.Name)); + h.MTime[0] = '0'; + h.UID[0] = '0'; + h.GID[0] = '0'; + memcpy(h.Mode, "644", 3); + h.Size[0] = '0'; + memcpy(h.Magic, "`\n", 2); + + write_chk(fd, &h, sizeof(h)); + + memset(&h, ' ', sizeof(h)); + memcpy(h.Name, "data.tar/ ", sizeof(h.Name)); + h.MTime[0] = '0'; + h.UID[0] = '0'; + h.GID[0] = '0'; + memcpy(h.Mode, "644", 3); + h.Size[0] = '0'; + memcpy(h.Magic, "`\n", 2); + + write_chk(fd, &h, sizeof(h)); + + memset(&h, ' ', sizeof(h)); + memcpy(h.Name, "control.tar/ ", sizeof(h.Name)); + h.MTime[0] = '0'; + h.UID[0] = '0'; + h.GID[0] = '0'; + memcpy(h.Mode, "644", 3); + memcpy(h.Size, "512", 3); + memcpy(h.Magic, "`\n", 2); + + write_chk(fd, &h, sizeof(h)); + union + { + struct TarHeader t; + unsigned char buf[512]; + } t; + for (unsigned int i = 0; i < sizeof(t.buf); i++) + t.buf[i] = '7'; + memcpy(t.t.Name, "control\0 ", 16); + memcpy(t.t.UserName, "userName", 8); + memcpy(t.t.GroupName, "thisIsAGroupNamethisIsAGroupName", 32); + t.t.LinkFlag = flag; + base256_encode(t.t.Size, size, sizeof(t.t.Size)); + memset(t.t.Checksum, ' ', sizeof(t.t.Checksum)); + + unsigned long sum = 0; + for (unsigned int i = 0; i < sizeof(t.buf); i++) + sum += t.buf[i]; + + int written = sprintf(t.t.Checksum, "%lo", sum); + for (unsigned int i = written; i < sizeof(t.t.Checksum); i++) + t.t.Checksum[i] = ' '; + + write_chk(fd, t.buf, sizeof(t.buf)); +} -// This does not trigger a vulnerability. It is just a basis for exploration. static void createdeb_test(const int fd) { // Magic number @@ -221,6 +295,14 @@ int main(int argc, char *argv[]) { createdeb_leakfd(fd); } + else if (strcmp(mode, "long-name") == 0) + { + createdeb_bigtarfilelength(fd, 'L'); + } + else if (strcmp(mode, "long-link") == 0) + { + createdeb_bigtarfilelength(fd, 'K'); + } else if (strcmp(mode, "test") == 0) { createdeb_test(fd); -- cgit v1.2.3 From 0444f9dd52c2bc7bec315f6f1ecad76a30713fa0 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Sat, 5 Dec 2020 20:17:56 +0100 Subject: CVE-2020-27350: debfile: integer overflow: Limit control size to 64 MiB Like the code in arfile.cc, MemControlExtract also has buffer overflows, in code allocating memory for parsing control files. Specify an upper limit of 64 MiB for control files to both protect against the Size overflowing (we allocate Size + 2 bytes), and protect a bit against control files consisting only of zeroes. --- apt-pkg/deb/debfile.cc | 15 +++++++++++++++ test/integration/test-cve-2020-27350 | 3 +++ test/interactive-helper/createdeb-cve-2020-27350.cc | 4 ++++ 3 files changed, 22 insertions(+) diff --git a/apt-pkg/deb/debfile.cc b/apt-pkg/deb/debfile.cc index f8d752e7f..bef0cd0d8 100644 --- a/apt-pkg/deb/debfile.cc +++ b/apt-pkg/deb/debfile.cc @@ -184,11 +184,23 @@ bool debDebFile::ControlExtract::DoItem(Item &Itm,int &Fd) // --------------------------------------------------------------------- /* This sets up to extract the control block member file into a memory block of just the right size. All other files go into the bit bucket. */ + +// Upper size limit for control files. Two reasons for having a limit here: +// +// 1. We read those files into memory and want to avoid being killed by OOM +// +// 2. We allocate (Itm.Size+2)-large arrays, so this can overflow if Itm.Size +// becomes 2**64-2 or larger. This is obviously +// +// 64 MiB seems like a terribly large size that everyone should be happy with. +static const unsigned long long DEB_CONTROL_SIZE_LIMIT = 64 * 1024 * 1024; bool debDebFile::MemControlExtract::DoItem(Item &Itm,int &Fd) { // At the control file, allocate buffer memory. if (Member == Itm.Name) { + if (Itm.Size > DEB_CONTROL_SIZE_LIMIT) + return _error->Error("Control file too large: %llu > %llu bytes", Itm.Size, DEB_CONTROL_SIZE_LIMIT); delete [] Control; Control = new char[Itm.Size+2]; IsControl = true; @@ -237,6 +249,9 @@ bool debDebFile::MemControlExtract::Read(debDebFile &Deb) record. */ bool debDebFile::MemControlExtract::TakeControl(const void *Data,unsigned long long Size) { + if (Size > DEB_CONTROL_SIZE_LIMIT) + return _error->Error("Control file too large: %llu > %llu bytes", Size, DEB_CONTROL_SIZE_LIMIT); + delete [] Control; Control = new char[Size+2]; Length = Size; diff --git a/test/integration/test-cve-2020-27350 b/test/integration/test-cve-2020-27350 index 336dc5b7e..f4bb79bcb 100755 --- a/test/integration/test-cve-2020-27350 +++ b/test/integration/test-cve-2020-27350 @@ -17,3 +17,6 @@ testequal "E: Long name to large: 67108865 bytes > 1048576 bytes" runapt ${APTTE ${APTTESTHELPERSBINDIR}/createdeb-cve-2020-27350 long-link long-link.deb testequal "E: Long name to large: 67108865 bytes > 1048576 bytes" runapt ${APTTESTHELPERSBINDIR}/extract-control long-link.deb control + +${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 diff --git a/test/interactive-helper/createdeb-cve-2020-27350.cc b/test/interactive-helper/createdeb-cve-2020-27350.cc index 7c58eb9df..af049d4e8 100644 --- a/test/interactive-helper/createdeb-cve-2020-27350.cc +++ b/test/interactive-helper/createdeb-cve-2020-27350.cc @@ -303,6 +303,10 @@ int main(int argc, char *argv[]) { createdeb_bigtarfilelength(fd, 'K'); } + else if (strcmp(mode, "long-control") == 0) + { + createdeb_bigtarfilelength(fd, '0'); + } else if (strcmp(mode, "test") == 0) { createdeb_test(fd); -- cgit v1.2.3 From df81895bce764dd02fbb4d67b92d28a730b5281f 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-pkg/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-pkg/contrib/extracttar.cc b/apt-pkg/contrib/extracttar.cc index f653e227e..dc2437528 100644 --- a/apt-pkg/contrib/extracttar.cc +++ b/apt-pkg/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 /*{{{*/ // --------------------------------------------------------------------- /* */ @@ -171,6 +176,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