Re: shell32: shlfileop tests: avoid buffer overflows for paths

[James Hawkins]

On 2/4/07, Mikołaj Zalewski <mikolaj(a)zalewski.pl> wrote:
> These buffers can contain multiple paths and set_curr_dir_path can make
> the paths long resulting in a buffer overflow.
>
> From 36e05f3140e57bad2af790c56073799d23f7ef60 Mon Sep 17 00:00:00 2001
> From: =?utf-8?q?Miko=C5=82aj_Zalewski?= <mikolaj(a)zalewski.pl>
> Date: Thu, 25 Jan 2007 12:17:11 +0100
> Subject: [PATCH] shell32: shlfileop tests: avoid buffer overflows for paths
>
> ---
>  dlls/shell32/tests/shlfileop.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/dlls/shell32/tests/shlfileop.c b/dlls/shell32/tests/shlfileop.c
> index b363c5e..23aaeba 100644
> --- a/dlls/shell32/tests/shlfileop.c
> +++ b/dlls/shell32/tests/shlfileop.c
> @@ -329,8 +329,8 @@ static void test_delete(void)
>  static void test_rename(void)
>  {
>      SHFILEOPSTRUCTA shfo, shfo2;
> -    CHAR from[MAX_PATH];
> -    CHAR to[MAX_PATH];
> +    CHAR from[5*MAX_PATH];
> +    CHAR to[5*MAX_PATH];
>      DWORD retval;
>
> shfo.hwnd = NULL;

This is really hacky.  If this is fixed, it needs to be fixed properly.

-- 
James Hawkins

shell32: shlfileop tests: avoid buffer overflows for paths

[Mikołaj Zalewski]

> > @@ -329,8 +329,8 @@ static void test_delete(void)
> >  static void test_rename(void)
> >  {
> >      SHFILEOPSTRUCTA shfo, shfo2;
> > -    CHAR from[MAX_PATH];
> > -    CHAR to[MAX_PATH];
> > +    CHAR from[5*MAX_PATH];
> > +    CHAR to[5*MAX_PATH];
> >      DWORD retval;
> >
> > shfo.hwnd = NULL;
>
> This is really hacky.  If this is fixed, it needs to be fixed properly.

I thought this is enough - we never put more that three paths in the 
buffer and tester probably won't have paths longer than MAX_PATH as they 
are not supported by the shell.
  What do you mean by a proper fix - some overflow checking in 
set_curr_dir_path and skipping the rest of tests in such a case?

Mikolaj Zalewski