[ros-diffs] [tkreuzer] 57970: [WIN32K] - Rewrite NtUserGetAtomName, make it return the length in WCHARs instead of bytes, as it is supposed to be. Fix a buffer overrun, when the caller passes a too large UNICO...

Author: tkreuzer
Date: Sat Dec 22 17:43:20 2012
New Revision: 57970

URL: http://svn.reactos.org/svn/reactos?rev=57970&view=rev
Log:
[WIN32K]
- Rewrite NtUserGetAtomName, make it return the length in WCHARs instead of bytes, as it
is supposed to be. Fix a buffer overrun, when the caller passes a too large UNICODE_STRING
buffer. Probe the output buffers before accessing them. Makes sure the target string is
always NULL terminated, even if the buffer is too small for the whole name.
- Remove NULL termination code from IntGetAtomName, since that is already done by
RtlQueryAtomInAtomTable

Modified:
    trunk/reactos/win32ss/include/ntuser.h
    trunk/reactos/win32ss/user/ntuser/useratom.c

Modified: trunk/reactos/win32ss/include/ntuser.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/include/ntuser.h?r…
==============================================================================
--- trunk/reactos/win32ss/include/ntuser.h [iso-8859-1] (original)
+++ trunk/reactos/win32ss/include/ntuser.h [iso-8859-1] Sat Dec 22 17:43:20 2012
@@ -1893,11 +1893,13 @@
 NtUserGetAsyncKeyState(
   INT Key);
 
-DWORD
-NTAPI
+_Success_(return!=0)
+_At_(pustrName->Buffer, _Out_z_bytecap_post_bytecount_(pustrName->MaximumLength,
return*2+2))
+ULONG
+APIENTRY
 NtUserGetAtomName(
-    ATOM nAtom,
-    PUNICODE_STRING pBuffer);
+    _In_ ATOM atom,
+    _Inout_ PUNICODE_STRING pustrName);
 
 UINT
 NTAPI

Modified: trunk/reactos/win32ss/user/ntuser/useratom.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/ntuser/userat…
==============================================================================
--- trunk/reactos/win32ss/user/ntuser/useratom.c [iso-8859-1] (original)
+++ trunk/reactos/win32ss/user/ntuser/useratom.c [iso-8859-1] Sat Dec 22 17:43:20 2012
@@ -34,11 +34,11 @@
 }
 
 ULONG FASTCALL
-IntGetAtomName(RTL_ATOM nAtom, LPWSTR lpBuffer, ULONG nSize)
+IntGetAtomName(RTL_ATOM nAtom, LPWSTR lpBuffer, ULONG cjBufSize)
 {
    NTSTATUS Status = STATUS_SUCCESS;
    PTHREADINFO pti;
-   ULONG Size = nSize;
+   ULONG Size = cjBufSize;
 
    pti = PsGetCurrentThreadWin32Thread();
    if (pti->rpdesk == NULL)
@@ -49,13 +49,12 @@
 
    Status = RtlQueryAtomInAtomTable(gAtomTable, nAtom, NULL, NULL, lpBuffer, &Size);
 
-   if (Size < nSize)
-      *(lpBuffer + Size/sizeof(WCHAR)) = 0;
    if (!NT_SUCCESS(Status))
    {
       SetLastNtError(Status);
       return 0;
    }
+
    return Size;
 }
 
@@ -78,30 +77,67 @@
    return Atom;
 }
 
-DWORD
+/*!
+ * \brief Returns the name of an atom.
+ *
+ * \param atom - The atom to be queried.
+ * \param pustrName - Pointer to an initialized UNICODE_STRING that receives
+ *                    the name of the atom. The function does not update the
+                      Length member. The string is always NULL-terminated.
+ *
+ * \return The length of the name in characters, or 0 if the function fails.
+ *
+ * \note The function does not aquire any global lock, since synchronisation is
+ *       handled by the RtlAtom function.
+ */
+_Success_(return!=0)
+_At_(pustrName->Buffer, _Out_z_bytecap_post_bytecount_(pustrName->MaximumLength,
return*2+2))
+ULONG
 APIENTRY
 NtUserGetAtomName(
-    ATOM nAtom,
-    PUNICODE_STRING pBuffer)
+    _In_ ATOM atom,
+    _Inout_ PUNICODE_STRING pustrName)
 {
-   DWORD Ret;
-   WCHAR Buffer[256];
-   UNICODE_STRING CapturedName = {0};
-   UserEnterShared();
-   CapturedName.Buffer = (LPWSTR)&Buffer;
-   CapturedName.MaximumLength = sizeof(Buffer);
-   Ret = IntGetAtomName((RTL_ATOM)nAtom, CapturedName.Buffer,
(ULONG)CapturedName.Length);
-   _SEH2_TRY
-   {
-      RtlCopyMemory(pBuffer->Buffer, &Buffer, pBuffer->MaximumLength);
-   }
-   _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
-   {
-      Ret = 0;
-   }
-   _SEH2_END
-   UserLeave();
-   return Ret;
+    WCHAR awcBuffer[256];
+    ULONG cjLength;
+
+    /* Retrieve the atom name into a local buffer (max length is 255 chars) */
+    cjLength = IntGetAtomName((RTL_ATOM)atom, awcBuffer, sizeof(awcBuffer));
+    if (cjLength != 0)
+    {
+        _SEH2_TRY
+        {
+            /* Probe the unicode string and the buffer */
+            ProbeForRead(pustrName, sizeof(*pustrName), 1);
+            ProbeForWrite(pustrName->Buffer, pustrName->MaximumLength, 1);
+
+            /* Check if we have enough space to write the NULL termination */
+            if (pustrName->MaximumLength >= sizeof(UNICODE_NULL))
+            {
+                /* Limit the length to the buffer size */
+                cjLength = min(pustrName->MaximumLength - sizeof(UNICODE_NULL),
+                               cjLength);
+
+                /* Copy the string and NULL terminate it */
+                RtlCopyMemory(pustrName->Buffer, awcBuffer, cjLength);
+                pustrName->Buffer[cjLength / sizeof(WCHAR)] = L'\0';
+            }
+            else
+            {
+                cjLength = 0;
+            }
+        }
+        _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
+        {
+            /* On exception, set last error and fail */
+            SetLastNtError(_SEH2_GetExceptionCode());
+            cjLength = 0;
+        }
+        _SEH2_END
+    }
+
+    /* Return the length in characters */
+    return cjLength / sizeof(WCHAR);
 }
 
 /* EOF */