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

Reply via email to