Re: [PATCH] cygwin: fix potential buffer overflow in fork

[Corinna Vinschen <corinna-cygwin at cygwin dot com>]

Hi Michael,

On Oct  9 18:58, Michael Haubenwallner wrote:
> When fork fails, we can use "%s" now with system_sprintf for the errmsg
> rather than a (potentially too small) buffer for the format string.

How could buf be too small?

> 
> * fork.cc (fork): Use "%s" with system_printf now.
> ---
>  winsup/cygwin/fork.cc | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc
> index 73a72f530..bcbef12d8 100644
> --- a/winsup/cygwin/fork.cc
> +++ b/winsup/cygwin/fork.cc
> @@ -618,13 +618,8 @@ fork ()
>        if (!grouped.errmsg)
>  	syscall_printf ("fork failed - child pid %d, errno %d", grouped.child_pid, grouped.this_errno);
>        else
> -	{
> -	  char buf[strlen (grouped.errmsg) + sizeof ("child %d - , errno 4294967295  ")];
> -	  strcpy (buf, "child %d - ");
> -	  strcat (buf, grouped.errmsg);
> -	  strcat (buf, ", errno %d");
> -	  system_printf (buf, grouped.child_pid, grouped.this_errno);
> -	}
> +	system_printf ("child %d - %s, errno %d", grouped.child_pid,
> +		       grouped.errmsg, grouped.this_errno);
>  
>        set_errno (grouped.this_errno);
>      }
> -- 
> 2.14.2

I guess this also means we can drop the if/else, kind of like

  system_printf ("child %d %s%s, errno %d",
		 grouped.child_pid,
		 grouped.errmsg ? "- " : "",
		 grouped.errmsg ?: "",
		 grouped.this_errno);

What do you think?


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

Attachment: signature.asc
Description: PGP signature