summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJulian Andres Klode <julian.klode@canonical.com>2020-12-04 12:37:19 +0100
committerJulian Andres Klode <julian.klode@canonical.com>2020-12-09 17:30:43 +0100
commit822db13d68658a1a20df2d19c688c18faa331616 (patch)
tree56d69c08a14cbc1e58b58b07231d9df3dd5cd4c7
parentd10c68d628fe5342d400a999a6d10c5c7c0cef41 (diff)
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.
-rw-r--r--apt-pkg/contrib/extracttar.cc11
-rwxr-xr-xtest/integration/test-cve-2020-273506
-rw-r--r--test/interactive-helper/createdeb-cve-2020-27350.cc84
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<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));
+}
-// 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);