[PATCH] comctl32: cchTextMax in TVN_ENDLABELEDIT should be the full buffer size

[Zhiyi Zhang]

> --- a/dlls/comctl32/tests/treeview.c
> +++ b/dlls/comctl32/tests/treeview.c
> @@ -1320,7 +1320,13 @@ static LRESULT CALLBACK parent_wnd_proc(HWND hWnd, UINT message, WPARAM wParam,
>                  break;
>                }
>  
> -            case TVN_ENDLABELEDITA: return TRUE;
> +            case TVN_ENDLABELEDITA:
> +              {
> +                NMTVDISPINFOA *disp = (NMTVDISPINFOA *)lParam;
> +                if (disp->item.mask & TVIF_TEXT)
> +                    ok(disp->item.cchTextMax == 260, "cchTextMax is %d\n", disp->item.cchTextMax);
You can replace 260 with MAX_PATH. You also need to test what happens if a string longer than MAX_PATH
was used, is cchTextMax still MAX_PATH?

> +                return TRUE;
> +              }
>              case TVN_ITEMEXPANDINGA:
>                {
>                  UINT newmask = pTreeView->itemNew.mask & ~TVIF_CHILDREN;
> diff --git a/dlls/comctl32/treeview.c b/dlls/comctl32/treeview.c
> index 3c73964304..afe16a0f21 100644
> --- a/dlls/comctl32/treeview.c
> +++ b/dlls/comctl32/treeview.c
> @@ -4017,7 +4017,7 @@ TREEVIEW_EndEditLabelNow(TREEVIEW_INFO *infoPtr, BOOL bCancel)
>  
>          tvdi.item.mask = TVIF_TEXT;
>      tvdi.item.pszText = tmpText;
> -    tvdi.item.cchTextMax = iLength + 1;
> +    tvdi.item.cchTextMax = TEXT_CALLBACK_SIZE;
You can remove previous iLength calculation since it's now a constant.

More to do:
1. Test that whether the message is using a constant buffer or a dynamic buffer that
can't be resize below MAX_PATH. If the buffer is of size MAX_PATH, tmpText can be shrinked.
2. Please consider and test what happens if an application replaces the pszText pointer
    with a different string pointer, with a longer string content.
3. Test ASCII and Unicode version of messages.

Thanks,
Zhiyi
>      }
>      else
>      {


On 11/10/19 10:26 AM, Damjan Jovanovic wrote:
> In Password Safe, when the user edits a tree view label, and removes
> brackets, the application wants to restore the original, longer
> string. It does this by editing pszText within the TVITEM.
> It determines the length of the buffer from cchTextMax. Windows passes
> 260 and all is well. Wine passes strlenW(pszText)+1, which is of
> minimal length, and trying to copy a longer string into it causes
> the MSVC runtime to falsely detect a buffer overflow and raise
> an exception, crashing the application.
>
> Let's pass 260 like Windows.
>
> Closes #16808.
>
> Signed-off-by: Damjan Jovanovic <damjan.jov at gmail.com>
> ---
>  dlls/comctl32/tests/treeview.c | 8 +++++++-
>  dlls/comctl32/treeview.c       | 2 +-
>  2 files changed, 8 insertions(+), 2 deletions(-)