Merge branch 'pu/cve-2020-27350'

6 files changed, 400 insertions, 2 deletions

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<unsigned long long>(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/apt-pkg/contrib/extracttar.cc b/apt-pkg/contrib/extracttar.cc
index 1616c9f12..dc2437528 100644
--- a/apt-pkg/contrib/extracttar.cc
+++ b/apt-pkg/contrib/extracttar.cc
@@ -55,7 +55,17 @@ 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;
+
+// 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					/*{{{*/
 // ---------------------------------------------------------------------
 /* */
@@ -166,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.
@@ -218,6 +233,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 +254,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/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
new file mode 100755
index 000000000..a32bf95e5
--- /dev/null
+++ b/test/integration/test-cve-2020-27350
@@ -0,0 +1,25 @@
+#!/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
+
+${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
+
+${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/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..8b9619469
--- /dev/null
+++ b/test/interactive-helper/createdeb-cve-2020-27350.cc
@@ -0,0 +1,325 @@
+#include <errno.h>
+#include <fcntl.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+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 = "!<arch>\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 = "!<arch>\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 = "!<arch>\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);
+}
+static void base256_encode(char *Str, unsigned long long Num, unsigned int Len)
+{
+   Str += Len;
+   while (Len-- > 0) {
+      *--Str = static_cast<char>(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 = "!<arch>\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));
+}
+
+static void createdeb_test(const int fd)
+{
+   // Magic number
+   static const char *magic = "!<arch>\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 <mode> <filename>\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, "long-name") == 0)
+   {
+      createdeb_bigtarfilelength(fd, 'L');
+   }
+   else if (strcmp(mode, "long-link") == 0)
+   {
+      createdeb_bigtarfilelength(fd, 'K');
+   }
+   else if (strcmp(mode, "long-control") == 0)
+   {
+      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);
+   }
+   else
+   {
+      fprintf(stderr, "Mode not recognized: %s\n", mode);
+   }
+
+   close(fd);
+   return EXIT_SUCCESS;
+}