Received: from vps892.directvps.nl (ikke.info [178.21.113.177])
	by nld3-dev1.alpinelinux.org (Postfix) with ESMTPS id EB4C3782B1C
	for <alpine-aports@lists.alpinelinux.org>; Mon, 28 Oct 2019 19:34:27 +0000 (UTC)
Received: by vps892.directvps.nl (Postfix, from userid 1008)
	id 79FC74400DB; Mon, 28 Oct 2019 20:34:27 +0100 (CET)
Date: Mon, 28 Oct 2019 20:34:27 +0100
From: Kevin Daudt <kdaudt@alpinelinux.org>
To: Nathan Owens <ndowens04@gmail.com>
Cc: alpine-aports@lists.alpinelinux.org
Subject: Re: [PATCH] main/file
Message-ID: <20191028193427.GE242707@alpha>
References: <20191022201140.1035-1-ndowens04@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20191022201140.1035-1-ndowens04@gmail.com>
User-Agent: Mutt/1.12.2 (2019-09-21)

Hellow Nathan,

Thanks for your contribution.

There are a couple of issues with this patch, see inline feedback.

Kind regards, Kevin

On Tue, Oct 22, 2019 at 08:11:40PM +0000, Nathan Owens wrote:
> CVE:https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-18218
> Patch:https://github.com/file/file/commit/46a8443f76cec4b41ec736eca396984c74664f84?diff=unified
> ---
>  main/file/APKBUILD             | 10 ++++--
>  main/file/CVE-2019-18218.patch | 59 ++++++++++++++++++++++++++++++++++
>  2 files changed, 67 insertions(+), 2 deletions(-)
>  create mode 100644 main/file/CVE-2019-18218.patch
> 
> diff --git a/main/file/APKBUILD b/main/file/APKBUILD
> index 433245e420..62d9e27554 100644
> --- a/main/file/APKBUILD
> +++ b/main/file/APKBUILD
> @@ -9,7 +9,8 @@ arch="all"

You are missing a pkgrel bump. Without it, the package is not going to
be rebuilt.

>  license="BSD-2-Clause"
>  makedepends="autoconf libtool automake"
>  subpackages="$pkgname-dev $pkgname-doc libmagic"
> -source=$pkgname-$pkgver.tar.gz::https://github.com/file/file/archive/FILE${pkgver/./_}.tar.gz
> +source="$pkgname-$pkgver.tar.gz::https://github.com/file/file/archive/FILE${pkgver/./_}.tar.gz
> +	CVE-2019-18218.patch"
>  builddir="$srcdir/$pkgname-FILE${pkgver/./_}"
>  
>  # secfixes:

Please include the CVE ID in the secfixes list here.

> @@ -20,6 +21,10 @@ builddir="$srcdir/$pkgname-FILE${pkgver/./_}"
>  #     - CVE-2019-8906
>  #     - CVE-2019-8907
>  
> +prepare() {
> +	#secfix CVE-2019-18218
> +	patch -p1 -i "$srcdir"/CVE-2019-18218.patch
> +}

abuild automatically applies patch files, no need to do it manually.

>  build() {
>  	SH_LIBTOOL='/usr/share/build-1/libtool' autoreconf -f -i
>  	./configure \
> @@ -44,4 +49,5 @@ libmagic() {
>  	mv "$pkgdir"/usr/lib "$pkgdir"/usr/share "$subpkgdir"/usr
>  }
>  
> -sha512sums="9b6ae3dd910a03d2161c91ebc75ac91eb7dbd279563462b77daf902d9ae9f0a70de12c37a498b20c6357d6594059d01841bfd104592107b65c08d8343fca19d2  file-5.37.tar.gz"
> +sha512sums="9b6ae3dd910a03d2161c91ebc75ac91eb7dbd279563462b77daf902d9ae9f0a70de12c37a498b20c6357d6594059d01841bfd104592107b65c08d8343fca19d2  file-5.37.tar.gz
> +62f9b2fc8f3daeeaa82b5f1fada4ebf84c149ff127b96d610ab210a2b581c5a29385d3b9267fbacc4cadb22ddfafeb5550aab6a926eba099c36e041e11b4a0e5  CVE-2019-18218.patch"
> diff --git a/main/file/CVE-2019-18218.patch b/main/file/CVE-2019-18218.patch
> new file mode 100644
> index 0000000000..f86e2709ea
> --- /dev/null
> +++ b/main/file/CVE-2019-18218.patch
> @@ -0,0 +1,59 @@
> +From b15ccbf355faa2203cccd4e29fa6206b1a1aa1b8 Mon Sep 17 00:00:00 2001
> +From: Christos Zoulas <christos@zoulas.com>
> +Date: Mon, 26 Aug 2019 14:31:39 +0000
> +Subject: [PATCH] Limit the number of elements in a vector (found by oss-fuzz)
> +
> +---
> + src/cdf.c | 9 ++++-----
> + src/cdf.h | 1 +
> + 2 files changed, 5 insertions(+), 5 deletions(-)
> +
> +diff --git a/src/cdf.c b/src/cdf.c
> +index 556a3ff8..e4835fef 100644
> +--- a/src/cdf.c
> ++++ b/src/cdf.c
> +@@ -35,7 +35,7 @@
> + #include "file.h"
> + 
> + #ifndef lint
> +-FILE_RCSID("@(#)$File: cdf.c,v 1.114 2019/02/20 02:35:27 christos Exp $")
> ++FILE_RCSID("@(#)$File: cdf.c,v 1.116 2019/08/26 14:31:39 christos Exp $")
> + #endif
> + 
> + #include <assert.h>
> +@@ -1013,8 +1013,9 @@ cdf_read_property_info(const cdf_stream_t *sst, const cdf_header_t *h,
> + 				goto out;
> + 			}
> + 			nelements = CDF_GETUINT32(q, 1);
> +-			if (nelements == 0) {
> +-				DPRINTF(("CDF_VECTOR with nelements == 0\n"));
> ++			if (nelements > CDF_ELEMENT_LIMIT || nelements == 0) {
> ++				DPRINTF(("CDF_VECTOR with nelements == %"
> ++				    SIZE_T_FORMAT "u\n", nelements));
> + 				goto out;
> + 			}
> + 			slen = 2;
> +@@ -1056,8 +1057,6 @@ cdf_read_property_info(const cdf_stream_t *sst, const cdf_header_t *h,
> + 					goto out;
> + 				inp += nelem;
> + 			}
> +-			DPRINTF(("nelements = %" SIZE_T_FORMAT "u\n",
> +-			    nelements));
> + 			for (j = 0; j < nelements && i < sh.sh_properties;
> + 			    j++, i++)
> + 			{
> +diff --git a/src/cdf.h b/src/cdf.h
> +index 2f7e554b..05056668 100644
> +--- a/src/cdf.h
> ++++ b/src/cdf.h
> +@@ -48,6 +48,7 @@
> + typedef int32_t cdf_secid_t;
> + 
> + #define CDF_LOOP_LIMIT					10000
> ++#define CDF_ELEMENT_LIMIT				100000
> + 
> + #define CDF_SECID_NULL					0
> + #define CDF_SECID_FREE					-1
> +-- 
> +2.23.0
> +
> -- 
> 2.23.0