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

[Damjan Jovanovic]

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 
---
 dlls/comctl32/tests/treeview.c | 8 +++++++-
 dlls/comctl32/treeview.c       | 2 +-
 2 files changed, 8 insertions(+), 2 deletions(-)

[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
>
> dlls/comctl32/tests/treeview.c | 8 +++++++-
>  dlls/comctl32/treeview.c       | 2 +-
>  2 files changed, 8 insertions(+), 2 deletions(-)