Package: zip
Version: 3.0-14
Severity: minor
Tags: patch

If zip 3.0-14 is built with _FORTIFY_SOURCE=3 (GCC 14.1, glibc 2.40),
this can happen when compressing a file with non-ASCII characters in its
UTF-8 name:

$ echo -n "There’s a Baby in the House.flac" | od -c
0000000   T   h   e   r   e 342 200 231   s       a       B   a   b   y
0000020       i   n       t   h   e       H   o   u   s   e   .   f   l
0000040   a   c
$ zip /tmp/t.zip "There’s a Baby in the House.flac"
*** buffer overflow detected ***: terminated

The problem is in local_to_wide_string, where mbstowcs is being run with
the UTF-8 source length rather than the widechar destination length --
this correctly trips a fortify error because GCC 14 can infer the actual
size of the destination.

I've attached a patch.

Thanks,

-- 
Adam Sampson <a...@offog.org>                         <http://offog.org/>
Avoid buffer overflow in local_to_wide_string.

The main problem here, which FORTIFY_SOURCE detects at runtime, was that
mbstowcs's size argument should be the size of the destination, not the
size of the source.

The two lines that add a terminating \0 were incorrect too. For the
first one, mbstowcs has either run out of space (in which case wsize
will be outside the bounds of the array), or it's succeeded (in which
case it's written the \0 itself). For the second one, the loop test will
have copied the \0 from the source string already.

--- zip-3.0-14/fileio.c 2008-05-29 01:13:24.000000000 +0100
+++ zip-3.0-14/fileio.c 2024-07-25 15:31:12.946353616 +0100
@@ -3487,7 +3487,7 @@
 zwchar *local_to_wide_string(local_string)
   char *local_string;
 {
-  int wsize;
+  size_t wsize, n;
   wchar_t *wc_string;
   zwchar *wide_string;
 
@@ -3502,15 +3502,18 @@
   if ((wc_string = (wchar_t *)malloc((wsize + 1) * sizeof(wchar_t))) == NULL) {
     ZIPERR(ZE_MEM, "local_to_wide_string");
   }
-  wsize = mbstowcs(wc_string, local_string, strlen(local_string) + 1);
-  wc_string[wsize] = (wchar_t) 0;
+  n = mbstowcs(wc_string, local_string, wsize + 1);
+  if (n != wsize) {
+    ZIPERR(ZE_LOGIC, "mbstowcs");
+  }
 
   /* in case wchar_t is not zwchar */
   if ((wide_string = (zwchar *)malloc((wsize + 1) * sizeof(zwchar))) == NULL) {
     ZIPERR(ZE_MEM, "local_to_wide_string");
   }
-  for (wsize = 0; (wide_string[wsize] = (zwchar)wc_string[wsize]); wsize++) ;
-  wide_string[wsize] = (zwchar)0;
+  for (n = 0; n < wsize + 1; n++) {
+    wide_string[n] = (zwchar)wc_string[n];
+  }
   free(wc_string);
 
   return wide_string;

Reply via email to