Re: [PATCH 1/4] regedit: Validate REG_SZ import data before processing it any further (v2)

[Alexandre Julliard]

Hugh McMaster <hugh.mcmaster(a)outlook.com> writes:
> Signed-off-by: Hugh McMaster <hugh.mcmaster(a)outlook.com>
> ---
>  programs/regedit/regproc.c       |  6 ++++++
>  programs/regedit/tests/regedit.c | 14 +++++++-------
>  2 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/programs/regedit/regproc.c b/programs/regedit/regproc.c
> index 96c4564..ccd9ebe 100644
> --- a/programs/regedit/regproc.c
> +++ b/programs/regedit/regproc.c
> @@ -293,6 +293,12 @@ static int REGPROC_unescape_string(WCHAR* str)
>                  str[val_idx] = str[str_idx];
>                  break;
>              }
> +        } else if (str[str_idx] == '"') {
> +            WCHAR *p = str + str_idx + 1;
> +            while (*p == ' ' || *p == '\t') p++;
> +            if (*p && *p != ';') return 0;
> +            str[val_idx++] = str[str_idx];
> +            break;

That would work, but it seems to me that the comment check would be
better in the caller, so that you can use the same code for validating
value names.

-- 
Alexandre Julliard
julliard(a)winehq.org

[PATCH 1/4] regedit: Validate REG_SZ import data before processing it any further (v2)

[Hugh McMaster]

On Friday, 21 April 2017 3:11 AM, Alexandre Julliard wrote:
> Hugh McMaster writes:
> > +        } else if (str[str_idx] == '"') {
> > +            WCHAR *p = str + str_idx + 1;
> > +            while (*p == ' ' || *p == '\t') p++;
> > +            if (*p && *p != ';') return 0;
> > +            str[val_idx++] = str[str_idx];
> > +            break;
>
> That would work, but it seems to me that the comment check would be
> better in the caller, so that you can use the same code for validating
> value names.

Sure. That would mean returning a pointer to any unparsed data in the original string.
It has to be the original string because converting escape sequences to their character
equivalents causes an offset, so relying on the new string length is unreliable.

Note, though, that the double quotes surrounding the value name are removed
before we call REGPROC_unescape_string(val_name) in processSetValue().

That said, if you want think it would be better to replace a lot of
the code in processSetValue() (see line 554 onwards in programs/regedit/regproc.c)
with REGPROC_unescape_string(), let me know.

Hugh

[PATCH 1/4] regedit: Validate REG_SZ import data before processing it any further (v2)

[Alexandre Julliard]

Hugh McMaster <hugh.mcmaster(a)outlook.com> writes:
> Sure. That would mean returning a pointer to any unparsed data in the original string.
> It has to be the original string because converting escape sequences to their character
> equivalents causes an offset, so relying on the new string length is unreliable.
>
> Note, though, that the double quotes surrounding the value name are removed
> before we call REGPROC_unescape_string(val_name) in processSetValue().
>
> That said, if you want think it would be better to replace a lot of
> the code in processSetValue() (see line 554 onwards in programs/regedit/regproc.c)
> with REGPROC_unescape_string(), let me know.

Yes, this should all be done through REGPROC_unescape_string(). It
should also handle removing the quotes, it doesn't make sense to
unescape but leave the quotes in.

-- 
Alexandre Julliard
julliard(a)winehq.org