Re: [ros-dev] [CONSRV] Ensure szMenuStringBack is NULL terminated to prevent possible buffer overread and fix char to wchar_t assignment.

[Thomas Faber]

On 2014-06-25 09:10, Vyronas Tsingaras wrote:
>   Is this patch acceptable? Please kindly explain any
> possible mistakes. 
The change to a wide character literal is correct, but a minor issue,
as these are identical integer values. To make the change consistent,
        ptrTab = wcschr(szMenuStringBack, L'\t');
and
            *ptrTab = UNICODE_NULL;
would be my preference.

I see two issues with szMenuStringBack, however your patch fixes
neither of them (and btw won't compile in ANSI C, you can't put
code before declarations):
(a) GetMenuItemInfoW can fail, leaving the buffer uninitialized -- no
    processing should be done on it in that case
(b) GetMenuItemInfoW can succeed, but the menu item text could fill up
    the whole buffer, leaving no space for the null -- null termination
    would have to be ensured (and/or mii.cch checked) _after_ the call
    to handle this case correctly
    (that is assuming our GetMenuItemInfoW is implemented correctly --
     if Windows's version always ensures null termination, this is not
     an issue here and should be fixed in that function instead)