Hi Santiago,
On Fri, Apr 25, 2025 at 08:25:38PM +0200, Santiago Vila wrote:
> In theory, this was already fixed by Fedora here:
>
> @@ -3502,7 +3502,7 @@
> 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 */
I don't think that's a correct fix. local_string here is the argument to
the function, so it can be of any length (e.g. an input filename).
mbstowcs returns the number of characters written to the buffer (see
mbcstowcs(3)). So if the converted version of local_string is too big to
fit in the buffer, it will return wsize + 1, and the assignment on the
next line will write outside the bounds of the buffer -- which is
potentially a security problem if this is being used with untrusted
input.
My patch added an error check here. You could also expand the buffer by
one more place so that there's always space to add a 0, but then it will
silently truncate the string which may cause other problems.
Thanks,
--
Adam Sampson <a...@offog.org> <http://offog.org/>
