From 66962a66970a1f816375620c89de7117a470a6af 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 --- test/integration/test-cve-2020-27350 | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100755 test/integration/test-cve-2020-27350 (limited to 'test/integration/test-cve-2020-27350') 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 -- cgit v1.2.3 From 29581d103fc85d988c1f8a9c995ef9a6bb600500 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. --- test/integration/test-cve-2020-27350 | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'test/integration/test-cve-2020-27350') 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 -- cgit v1.2.3 From ed786183bfe9813cf9bc603de0a2beed3e0d0e31 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. --- test/integration/test-cve-2020-27350 | 3 +++ 1 file changed, 3 insertions(+) (limited to 'test/integration/test-cve-2020-27350') 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 -- cgit v1.2.3 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. --- test/integration/test-cve-2020-27350 | 3 +++ 1 file changed, 3 insertions(+) (limited to 'test/integration/test-cve-2020-27350') 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 -- cgit v1.2.3