On Sun, Jan 12, 2025 at 05:02:30PM +0100, Theo Buehler wrote:
> On Sun, Jan 12, 2025 at 10:11:47PM +0900, SASANO Takayoshi wrote:
> > Hi,
> >
> > I could not extract GenEi-Latin font (TTF archive) at
> > https://okoneya.jp/font/GenEiLatin-Separate_v2.1.zip with unzip-6.0p17,
> > and I found "unzip: backwards memcpy" in /var/log messages.
> >
> > Remedy is simple, add -DNOMEMCPY.
>
> It's the memcpy in inflate.c. The (unsigned) cast is wrong. If d is
> larger than w, unsigned overflow will make the result of (unsigned)w - d
> larger than UINT_MAX - d, which will almost certainly be larger than e.
Ugh. Kudos for trying to grok this code.
> However, we can't just do memmove() unconditionally, because the
> fallback in case that w - d < e isn't actually memmove() or memcpy()...
> Yuck.
...
> I think this patch is safer than adding -DNOMEMCPY to the CFLAGS since
> it will keep using the same tested codepaths as before except that it
> no longer crashes on your example file (the resulting extracted files
> are the same).
Looking at the comment for NOMEMCPY in inflate.c, it's quite possible
it was added precisely to work around this bug.
ok jca@
> The check in explode.c seems correct since it doesn't have this cast.
>
> Index: Makefile
> ===================================================================
> RCS file: /cvs/ports/archivers/unzip/Makefile,v
> diff -u -p -r1.71 Makefile
> --- Makefile 7 Nov 2023 14:19:18 -0000 1.71
> +++ Makefile 12 Jan 2025 15:14:43 -0000
> @@ -5,7 +5,7 @@ COMMENT = extract, list & test files in
> VERSION = 6.0
> DISTNAME = unzip${VERSION:S/.//}
> PKGNAME = unzip-${VERSION}
> -REVISION = 17
> +REVISION = 18
> CATEGORIES = archivers
> SITES = ${SITE_SOURCEFORGE:=infozip/} \
> ftp://ftp.info-zip.org/pub/infozip/src/
> Index: patches/patch-inflate_c
> ===================================================================
> RCS file: patches/patch-inflate_c
> diff -N patches/patch-inflate_c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-inflate_c 12 Jan 2025 15:35:45 -0000
> @@ -0,0 +1,14 @@
> +Avoid backward memcpy
> +
> +Index: inflate.c
> +--- inflate.c.orig
> ++++ inflate.c
> +@@ -1018,7 +1018,7 @@ unsigned bl, bd; /* number of bits decoded by t
> + if ((unsigned)w - d >= e)
> + /* (this test assumes unsigned comparison) */
> + {
> +- memcpy(redirSlide + (unsigned)w, redirSlide + d, e);
> ++ memmove(redirSlide + (unsigned)w, redirSlide + d, e);
> + w += e;
> + d += e;
> + }
> Index: patches/patch-unix_zipgrep
> ===================================================================
> RCS file: /cvs/ports/archivers/unzip/patches/patch-unix_zipgrep,v
> diff -u -p -r1.1 patch-unix_zipgrep
> --- patches/patch-unix_zipgrep 14 Apr 2022 21:23:24 -0000 1.1
> +++ patches/patch-unix_zipgrep 12 Jan 2025 15:12:44 -0000
> @@ -1,10 +1,9 @@
> -Index: unix/zipgrep
> -
> CVE-2022-1271
> Prevent arbitrary-file-write vulnerability if an archive member
> name contains shell wild cards that expand to an existing pathname
> with an exploit pattern.
>
> +Index: unix/zipgrep
> --- unix/zipgrep.orig
> +++ unix/zipgrep
> @@ -70,7 +70,7 @@ for i in `unzip -Z1 "$zipfile" ${1+"$@"} | sed -e 's/\
>
--
jca