Re: [ros-dev] [ros-diffs] [jimtabor] 38518: - Removed SEH abuse and add notes for the hook code, in NtUserDefSetText. - Update NtUserCallHwndLock subfunctions.

[Thomas Bluemel]

ProbeForReadUnicodeString should at least probe the buffers, otherwise 
the function is pointless. I believe at one point it did, and it was 
probably removed for some strange reason. The reason it copies the 
UNICODE_STRING is so that the pointers can't be modified anymore.

Thomas

Timo Kreuzer wrote:
>   SEH is still needed. SafeText doesn't really
> deserve it's name, as it's
>  only a safe copy of the UNICODE_STRING structure  returned by
>  ProbeForReadUnicodeString(), but with the still unsafe string buffer.
>  Also the Buffer was never probed (ProbeForReadUnicodeString only checks
>  the UNICODE_STRING and copies it)
>
>  IMO the function is dangerous, as it implies that the Buffer was probed,
>  too.
>
>  Timo
>
>
>  jimtabor(a)svn.reactos.org schrieb:
>    
> >   Author: jimtabor
> >  Date: Fri Jan  2 22:02:54 2009
> >  New Revision: 38518
> >
> >  URL: http://svn.reactos.org/svn/reactos?rev=38518&view=rev
> >  Log:
> >  - Removed SEH abuse and add notes for the hook code, in NtUserDefSetText.
> >  - Update NtUserCallHwndLock subfunctions.
> >
> >  Modified:
> >      trunk/reactos/subsystems/win32/win32k/ntuser/simplecall.c
> >      trunk/reactos/subsystems/win32/win32k/ntuser/window.c
> >
> >    
> >       
>  ...
>    
> >   Modified:
> > trunk/reactos/subsystems/win32/win32k/ntuser/window.c
> >  URL:
> > http://svn.reactos.org/svn/reactos/trunk/reactos/subsystems/win32/win32k/nt…
> >  ==============================================================================
> >  --- trunk/reactos/subsystems/win32/win32k/ntuser/window.c [iso-8859-1] (original)
> >  +++ trunk/reactos/subsystems/win32/win32k/ntuser/window.c [iso-8859-1] Fri Jan  2
> > 22:02:54 2009
> >  @@ -4569,57 +4569,47 @@
> >      }
> >      Wnd = Window->Wnd;
> >   
> >  -   if(SafeText.Length != 0)
> >  -   {
> >  -      _SEH2_TRY
> >  -      {
> >  -          if (Wnd->WindowName.MaximumLength > 0 &&
> >  -              SafeText.Length <= Wnd->WindowName.MaximumLength -
> > sizeof(UNICODE_NULL))
> >  -          {
> >  -              ASSERT(Wnd->WindowName.Buffer != NULL);
> >  -
> >  -              Wnd->WindowName.Length = SafeText.Length;
> >  -              Wnd->WindowName.Buffer[SafeText.Length / sizeof(WCHAR)] =
> > L'\0';
> >  -              RtlCopyMemory(Wnd->WindowName.Buffer,
> >  -                            SafeText.Buffer,
> >  -                            SafeText.Length);
> >  -          }
> >  -          else
> >  -          {
> >  -              PWCHAR buf;
> >  -              Wnd->WindowName.MaximumLength = Wnd->WindowName.Length = 0;
> >  -              buf = Wnd->WindowName.Buffer;
> >  -              Wnd->WindowName.Buffer = NULL;
> >  -              if (buf != NULL)
> >  -              {
> >  -                  DesktopHeapFree(Wnd->pdesktop,
> >  -                                  buf);
> >  -              }
> >  -
> >  -              Wnd->WindowName.Buffer = DesktopHeapAlloc(Wnd->pdesktop,
> >  -                                                        SafeText.Length +
> > sizeof(UNICODE_NULL));
> >  -              if (Wnd->WindowName.Buffer != NULL)
> >  -              {
> >  -                  Wnd->WindowName.Buffer[SafeText.Length / sizeof(WCHAR)] =
> > L'\0';
> >  -                  RtlCopyMemory(Wnd->WindowName.Buffer,
> >  -                                SafeText.Buffer,
> >  -                                SafeText.Length);
> >  -                  Wnd->WindowName.MaximumLength = SafeText.Length +
> > sizeof(UNICODE_NULL);
> >  -                  Wnd->WindowName.Length = SafeText.Length;
> >  -              }
> >  -              else
> >  -              {
> >  -                  SetLastWin32Error(ERROR_NOT_ENOUGH_MEMORY);
> >  -                  Ret = FALSE;
> >  -              }
> >  -          }
> >  -      }
> >  -      _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
> >  -      {
> >  -           SetLastNtError(_SEH2_GetExceptionCode());
> >  -           Ret = FALSE;
> >  -      }
> >  -      _SEH2_END;
> >  +   if (SafeText.Length != 0)
> >  +   {
> >  +      if (Wnd->WindowName.MaximumLength > 0 &&
> >  +          SafeText.Length <= Wnd->WindowName.MaximumLength -
> > sizeof(UNICODE_NULL))
> >  +      {
> >  +         ASSERT(Wnd->WindowName.Buffer != NULL);
> >  +
> >  +         Wnd->WindowName.Length = SafeText.Length;
> >  +         Wnd->WindowName.Buffer[SafeText.Length / sizeof(WCHAR)] = L'\0';
> >  +         RtlCopyMemory(Wnd->WindowName.Buffer,
> >  +                              SafeText.Buffer,
> >  +                              SafeText.Length);
> >  +      }
> >  +      else
> >  +      {
> >  +         PWCHAR buf;
> >  +         Wnd->WindowName.MaximumLength = Wnd->WindowName.Length = 0;
> >  +         buf = Wnd->WindowName.Buffer;
> >  +         Wnd->WindowName.Buffer = NULL;
> >  +         if (buf != NULL)
> >  +         {
> >  +            DesktopHeapFree(Wnd->pdesktop, buf);
> >  +         }
> >  +
> >  +         Wnd->WindowName.Buffer = DesktopHeapAlloc(Wnd->pdesktop,
> >  +                                                   SafeText.Length +
> > sizeof(UNICODE_NULL));
> >  +         if (Wnd->WindowName.Buffer != NULL)
> >  +         {
> >  +            Wnd->WindowName.Buffer[SafeText.Length / sizeof(WCHAR)] = L'\0';
> >  +            RtlCopyMemory(Wnd->WindowName.Buffer,
> >  +                                 SafeText.Buffer,
> >  +                                 SafeText.Length);
> >  +            Wnd->WindowName.MaximumLength = SafeText.Length + sizeof(UNICODE_NULL);
> >  +            Wnd->WindowName.Length = SafeText.Length;
> >  +         }
> >  +         else
> >  +         {
> >  +            SetLastWin32Error(ERROR_NOT_ENOUGH_MEMORY);
> >  +            Ret = FALSE;
> >  +         }
> >  +      }
> >      }
> >      else
> >      {
> >  @@ -4628,6 +4618,9 @@
> >             Wnd->WindowName.Buffer[0] = L'\0';
> >      }
> >   
> >  +   // HAX! FIXME! Windows does not do this in here!
> >  +   // In User32, these are called after: NotifyWinEvent EVENT_OBJECT_NAMECHANGE than
> >  +   // RepaintButton, StaticRepaint, NtUserCallHwndLock
> > HWNDLOCK_ROUTINE_REDRAWFRAMEANDHOOK, etc.
> >      /* Send shell notifications */
> >      if (!IntGetOwner(Window) && !IntGetParent(Window))
> >      {
> >
> >
> >    
> >       
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev
>