zip Buffer Overflow Detected with Multibyte Filename and Fortified glibc

#How to Reproduce

Compile a zip on Linux with gcc 12 or newer, optimization O1 or more, and -D_FORTIFY_SOURCE=3. Create a file with a multibyte filename, and try to compress it. For example:

$ touch 汉字
$ LC_CTYPE="en_US.UTF-8" ./zip foo.zip 汉字
*** buffer overflow detected ***: terminated


zip error: Interrupted (aborting)

#How It Happened

When Unicode support is enabled, zip processes file names with the function local_to_wide_string. The function calculates the size of the converted wide string, allocates a buffer with that size, and puts the converted string into the buffer, as is shown in the following simplified code:

zwchar *local_to_wide_string(local_string)
  char *local_string;
{
  int wsize = mbstowcs(NULL, local_string, MB_CUR_MAX );
  wchar_t *wc_string = (wchar_t *)malloc((wsize + 1) * sizeof(wchar_t));
  wsize = mbstowcs(wc_string, local_string, strlen(local_string) + 1);
  // rest of function body omitted
}

Recent versions of glibc provides a FORTIFY_SOURCE option that enables additional parameter checks for certain functions, including mbstowcs. gcc 12 provides a builtin_dynamic_object_size that returns the size of a dynamically allocated object. Recall that for mbstowcs, the third parameter marks the maximum number of wide chars written to the destination array. When builtin_dynamic_object_size is available, fortified mbstowcs will check whether the third parameter is no more than the dynamic size of the first parameter and report buffer overflow detected when not.

Let's come back to local_to_wide_string function. When the input local_string contains a multibyte character, in the second mbstowcs call, the third parameter is strlen(local_string) + 1, but the destination wc_string is only allocated of size wsize + 1, thus failed to pass the fortified mbstowcs check.

Note that no actual buffer overflow would happen because there are only wsize elements that can be written to wc_string.

#Suggested Fix

diff --git a/fileio.c b/fileio.c
index 1847e62..5a2959d 100644
--- a/fileio.c
+++ b/fileio.c
@@ -3502,7 +3502,7 @@ zwchar *local_to_wide_string(local_string)
   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);
+  wsize = mbstowcs(wc_string, local_string, wsize + 1);
   wc_string[wsize] = (wchar_t) 0;
 
   /* in case wchar_t is not zwchar */

#Conclusion

This bug was reported to Info-ZIP.

But hey dude, why bother using zip?