Re: [PATCH] services: Avoid buffer overruns in test_runner and START_TEST.

[Gerald Pfeifer]

I never got a response to my patch below (and, yes, I checked the
list archives).

More than two years later the following, essentially identical, 
patch by Rémi was applied:

  commit bcbe1d120cf6f68dd6a888b488050b3db33d1e5c
  Author: Rémi Bernon <rbernon(a)codeweavers.com>
  Date:   Tue Feb 11 19:09:33 2020 +0100

    services/tests: Fix some format-overflow warnings.
    
    Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
    Signed-off-by: Alexandre Julliard <julliard(a)winehq.org>

Hmm...

Gerald

On Mon, 25 Dec 2017, Gerald Pfeifer wrote:
> A few days ago my GCC-based builder started picking this up, and
> looking into the code there is potential for an actual buffer overrun, 
> since service_name is included into named_pipe_name together with some
> constants, and both originally were the same size.
>
> This fixes it by increasing the size of the second buffer which also
> addresses the following warnings issued by GCC:
>
> service.c: In function ‘test_runner’:
> service.c:541:46: warning: ‘_pipe’ directive writing 5 bytes into a region 
> of size between 1 and 100 [-Wformat-overflow=]
>      sprintf(named_pipe_name, "\\\\.\\pipe\\%s_pipe", service_name);
>                                               ^~~~~
> service.c:541:5: note: ‘sprintf’ output between 15 and 114 bytes into a 
> destination of size ...
>      sprintf(named_pipe_name, "\\\\.\\pipe\\%s_pipe", service_name);
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> service.c: In function ‘func_service’:
> service.c:593:50: warning: ‘_pipe’ directive writing 5 bytes into a region 
> of size between 1 and 100 [-Wformat-overflow=]
>          sprintf(named_pipe_name, "\\\\.\\pipe\\%s_pipe", service_name);
>                                                   ^~~~~
> service.c:593:9: note: ‘sprintf’ output between 15 and 114 bytes into a 
> destination of size ...
>          sprintf(named_pipe_name, "\\\\.\\pipe\\%s_pipe", service_name);
>          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Gerald
>
> Signed-off-by: Gerald Pfeifer <gerald(a)pfeifer.com>
> ---
>  programs/services/tests/service.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/programs/services/tests/service.c b/programs/services/tests/service.c
> index 98419497a5..47adb1a397 100644
> --- a/programs/services/tests/service.c
> +++ b/programs/services/tests/service.c
> @@ -29,7 +29,8 @@
>  static SERVICE_STATUS_HANDLE (WINAPI *pRegisterServiceCtrlHandlerExA)(LPCSTR,LPHANDLER_FUNCTION_EX,LPVOID);
>
> static HANDLE pipe_handle = INVALID_HANDLE_VALUE;
> -static char service_name[100], named_pipe_name[100];
> +static char service_name[100],
> +            named_pipe_name[114]; /* will include service_name later on */
>  static SERVICE_STATUS_HANDLE service_handle;
>
> /* Service process global variables */