[PATCH] msvcrt: fix character/byte confusion in buffer overflow branch

[Juan Lang]

In general, I think you want to send this to wine-patches, not here.

On Mon, May 6, 2013 at 12:26 PM, Max Kellermann <max at duempel.org> wrote:

> The first memcpy() call in puts_clbk_str_w() confuses character count
> and byte count.  It uses the number of characters (out->len) as number
> of bytes.  This leaves half of the buffer undefined.
>
> Interestingly, the second memcpy() call in the same function is
> correct.
>
> This bug potentially makes applications expose internal (secret) data.
> Usually, the destination buffer is on the stack, and the stack often
> contains secrets.  Therefore, one could argue that this bug
> constitutes a security vulnerability.
>

It'd be hard to make that argument convincingly. That's neither here nor
there, I suppose, but...

---
>  dlls/msvcrt/printf.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/dlls/msvcrt/printf.h b/dlls/msvcrt/printf.h
> index cfba4b7..8b749bc 100644
> --- a/dlls/msvcrt/printf.h
> +++ b/dlls/msvcrt/printf.h
> @@ -48,7 +48,7 @@ static int FUNC_NAME(puts_clbk_str)(void *ctx, int len,
> const APICHAR *str)
>          return len;
>
>      if(out->len < len) {
> -        memcpy(out->buf, str, out->len);
> +        memcpy(out->buf, str, out->len*sizeof(APICHAR));
>          out->buf += out->len;
>

If the memcpy was incorrect, the += is also incorrect. I'm not sure which
is the case, but either way, your patch can't be correct as is.
--Juan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20130507/c26cac7c/attachment.html>