[ros-diffs] [jmorlan] 34413: Some fixes for console alias functions: - Pass buffers via a CSR capture buffer, instead of trying to cram them in the size-limited LPC message. - GetConsoleAliasW: Return number of bytes written, not unrelated "Size" variable. - GetConsoleAliasExesW: Return value is in bytes, not characters. - GetConsoleAliasA, GetConsoleAliasExesA: Parameters and returns of corresponding W functions are in bytes, not characters. - IntFindAliasHeader, IntGetAliasEntry: Break when current name i

Author: jmorlan
Date: Thu Jul 10 10:43:06 2008
New Revision: 34413

URL: http://svn.reactos.org/svn/reactos?rev=34413&view=rev
Log:
Some fixes for console alias functions:
- Pass buffers via a CSR capture buffer, instead of trying to cram them in the
size-limited LPC message.
- GetConsoleAliasW: Return number of bytes written, not unrelated "Size"
variable.
- GetConsoleAliasExesW: Return value is in bytes, not characters.
- GetConsoleAliasA, GetConsoleAliasExesA: Parameters and returns of corresponding W
functions are in bytes, not characters.
- IntFindAliasHeader, IntGetAliasEntry: Break when current name is greater, not less.
- IntCreateAliasHeader: Fix bad use of pointer arithmetic; initialize Data to NULL.
- IntCreateAliasEntry: Fix bad use of pointer arithmetic.
- IntGetConsoleAliasesExesLength: Fix infinite loop; add sizeof(WCHAR) instead of 1.
- IntGetAllConsoleAliasesLength: Fix infinite loop.
- CsrGetConsoleAlias, CsrGetAllConsoleAliases, CsrGetConsoleAliasesExes: Don't use a
winerror where an NTSTATUS is needed.

Modified:
    trunk/reactos/dll/win32/kernel32/misc/console.c
    trunk/reactos/subsystems/win32/csrss/api/alias.c

Modified: trunk/reactos/dll/win32/kernel32/misc/console.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/misc/co…
==============================================================================
--- trunk/reactos/dll/win32/kernel32/misc/console.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/kernel32/misc/console.c [iso-8859-1] Thu Jul 10 10:43:06 2008
@@ -327,13 +327,13 @@
 		  LPWSTR	lpExeName)
 {
   PCSR_API_MESSAGE Request; 
+  PCSR_CAPTURE_BUFFER CaptureBuffer;
   ULONG CsrRequest;
   NTSTATUS Status;
   ULONG Size;
   ULONG ExeLength;
   ULONG SourceLength;
   ULONG RequestLength;
-  //PVOID CaptureBuffer;
   WCHAR * Ptr;
 
   DPRINT("GetConsoleAliasW entered lpSource %S lpExeName %S\n", lpSource,
lpExeName);
@@ -343,12 +343,11 @@
   ExeLength = wcslen(lpExeName) + 1;
   SourceLength = wcslen(lpSource) + 1;
 
-  Size = (ExeLength + SourceLength + CSRSS_MAX_ALIAS_TARGET_LENGTH) * sizeof(WCHAR);
+  Size = (ExeLength + SourceLength) * sizeof(WCHAR);
 
   RequestLength = Size + sizeof(CSR_API_MESSAGE);
   Request = RtlAllocateHeap(GetProcessHeap(), 0, RequestLength);
   
-#if 0  
   CaptureBuffer = CsrAllocateCaptureBuffer(1, TargetBufferLength);
   if (!CaptureBuffer)
   {
@@ -363,36 +362,32 @@
                           (PVOID*)&Request->Data.GetConsoleAlias.TargetBuffer);
   Request->Data.GetConsoleAlias.TargetBufferLength = TargetBufferLength;
 
-#endif
-
   Ptr = (LPWSTR)((ULONG_PTR)Request + sizeof(CSR_API_MESSAGE));
   wcscpy(Ptr, lpSource);
   Ptr += SourceLength;
   wcscpy(Ptr, lpExeName);
-  Ptr += ExeLength;
 
   Request->Data.GetConsoleAlias.ExeLength = ExeLength;
-  Request->Data.GetConsoleAlias.TargetBufferLength = CSRSS_MAX_ALIAS_TARGET_LENGTH *
sizeof(WCHAR);
   Request->Data.GetConsoleAlias.SourceLength = SourceLength;
 
   Status = CsrClientCallServer(Request,
-			       NULL, //CaptureBuffer,
+			       CaptureBuffer,
 			       CsrRequest,
 			       sizeof(CSR_API_MESSAGE) + Size);
 
   if (!NT_SUCCESS(Status) || !NT_SUCCESS(Status = Request->Status))
   {
     RtlFreeHeap(GetProcessHeap(), 0, Request);
-    //CsrFreeCaptureBuffer(CaptureBuffer);
+    CsrFreeCaptureBuffer(CaptureBuffer);
     SetLastErrorByStatus(Status);
     return 0;
   }
 
-  wcscpy(lpTargetBuffer, Ptr);
+  wcscpy(lpTargetBuffer, Request->Data.GetConsoleAlias.TargetBuffer);
   RtlFreeHeap(GetProcessHeap(), 0, Request);
-  //CsrFreeCaptureBuffer(CaptureBuffer);
-
-  return Size;
+  CsrFreeCaptureBuffer(CaptureBuffer);
+
+  return Request->Data.GetConsoleAlias.BytesWritten;
 }
 
 
@@ -424,13 +419,13 @@
 
   lpwTargetBuffer = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, TargetBufferLength *
sizeof(WCHAR));
 
-  dwResult = GetConsoleAliasW(lpwSource, lpwTargetBuffer, TargetBufferLength,
lpwExeName);
+  dwResult = GetConsoleAliasW(lpwSource, lpwTargetBuffer, TargetBufferLength *
sizeof(WCHAR), lpwExeName);
 
   HeapFree(GetProcessHeap(), 0, lpwSource);
   HeapFree(GetProcessHeap(), 0, lpwExeName);
 
   if (dwResult)
-     dwResult = WideCharToMultiByte(CP_ACP, 0, lpwTargetBuffer, dwResult, lpTargetBuffer,
TargetBufferLength, NULL, NULL);
+     dwResult = WideCharToMultiByte(CP_ACP, 0, lpwTargetBuffer, dwResult / sizeof(WCHAR),
lpTargetBuffer, TargetBufferLength, NULL, NULL);
 
   HeapFree(GetProcessHeap(), 0, lpwTargetBuffer);
 
@@ -446,27 +441,42 @@
 		      DWORD	ExeNameBufferLength)
 {
   CSR_API_MESSAGE Request; 
+  PCSR_CAPTURE_BUFFER CaptureBuffer;
   ULONG CsrRequest;
   NTSTATUS Status;
 
   DPRINT("GetConsoleAliasExesW entered\n");
 
+  CaptureBuffer = CsrAllocateCaptureBuffer(1, ExeNameBufferLength);
+  if (!CaptureBuffer)
+  {
+    SetLastError(ERROR_NOT_ENOUGH_MEMORY);
+    return 0;
+  }
+
   CsrRequest = MAKE_CSR_API(GET_CONSOLE_ALIASES_EXES, CSR_NATIVE);
-  Request.Data.GetConsoleAliasesExes.ExeNames = lpExeNameBuffer;
+  CsrAllocateMessagePointer(CaptureBuffer,
+                            ExeNameBufferLength,
+                            (PVOID*)&Request.Data.GetConsoleAliasesExes.ExeNames);
   Request.Data.GetConsoleAliasesExes.Length = ExeNameBufferLength;
 
   Status = CsrClientCallServer(& Request,
-			       NULL,
+			       CaptureBuffer,
 			       CsrRequest,
 			       sizeof(CSR_API_MESSAGE));
 
   if (!NT_SUCCESS(Status) || !NT_SUCCESS(Status = Request.Status))
   {
     SetLastErrorByStatus(Status);
+    CsrFreeCaptureBuffer(CaptureBuffer);
     return 0;
   }
 
-  return Request.Data.GetConsoleAliasesExes.BytesWritten / sizeof(WCHAR);
+  memcpy(lpExeNameBuffer,
+         Request.Data.GetConsoleAliasesExes.ExeNames,
+         Request.Data.GetConsoleAliasesExes.BytesWritten);
+  CsrFreeCaptureBuffer(CaptureBuffer);
+  return Request.Data.GetConsoleAliasesExes.BytesWritten;
 }
 
 
@@ -484,10 +494,10 @@
 
   lpwExeNameBuffer = HeapAlloc(GetProcessHeap(), 0, ExeNameBufferLength *
sizeof(WCHAR));
 
-  dwResult = GetConsoleAliasExesW(lpwExeNameBuffer, ExeNameBufferLength);
+  dwResult = GetConsoleAliasExesW(lpwExeNameBuffer, ExeNameBufferLength *
sizeof(WCHAR));
 
   if (dwResult)
-      dwResult = WideCharToMultiByte(CP_ACP, 0, lpwExeNameBuffer, dwResult,
lpExeNameBuffer, ExeNameBufferLength, NULL, NULL);
+      dwResult = WideCharToMultiByte(CP_ACP, 0, lpwExeNameBuffer, dwResult /
sizeof(WCHAR), lpExeNameBuffer, ExeNameBufferLength, NULL, NULL);
 
   HeapFree(GetProcessHeap(), 0, lpwExeNameBuffer);
   return dwResult;

Modified: trunk/reactos/subsystems/win32/csrss/api/alias.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/subsystems/win32/csrss/api…
==============================================================================
--- trunk/reactos/subsystems/win32/csrss/api/alias.c [iso-8859-1] (original)
+++ trunk/reactos/subsystems/win32/csrss/api/alias.c [iso-8859-1] Thu Jul 10 10:43:06
2008
@@ -34,6 +34,21 @@
 
 static PALIAS_HEADER RootHeader = NULL;
 
+/* Ensure that a buffer is contained within the process's shared memory section. */
+static BOOL
+ValidateBuffer(PCSRSS_PROCESS_DATA ProcessData, PVOID Buffer, ULONG Size)
+{
+    ULONG Offset = (BYTE *)Buffer - (BYTE *)ProcessData->CsrSectionViewBase;
+    if (Offset >= ProcessData->CsrSectionViewSize
+        || Size > (ProcessData->CsrSectionViewSize - Offset))
+    {
+        DPRINT1("Invalid buffer %p %d; not within %p %d\n",
+            Buffer, Size, ProcessData->CsrSectionViewBase,
ProcessData->CsrSectionViewSize);
+        return FALSE;
+    }
+    return TRUE;
+}
+
 static
 PALIAS_HEADER
 IntFindAliasHeader(PALIAS_HEADER RootHeader, LPCWSTR lpExeName)
@@ -44,7 +59,7 @@
         if (!diff)
             return RootHeader;
 
-        if (diff < 0)
+        if (diff > 0)
             break;
 
         RootHeader = RootHeader->Next;
@@ -62,8 +77,9 @@
   if (!Entry)
       return Entry;
   
-  Entry->lpExeName = (LPCWSTR)(Entry + sizeof(ALIAS_HEADER));
+  Entry->lpExeName = (LPCWSTR)(Entry + 1);
   wcscpy((WCHAR*)Entry->lpExeName, lpExeName);
+  Entry->Data = NULL;
   Entry->Next = NULL;
   return Entry;
 }
@@ -117,7 +133,7 @@
         if (!diff)
             return RootHeader;
 
-        if (diff < 0)
+        if (diff > 0)
             break;
 
         RootHeader = RootHeader->Next;
@@ -175,7 +191,7 @@
    if (!Entry)
        return Entry;
 
-   Entry->lpSource = (LPCWSTR)(Entry + sizeof(ALIAS_ENTRY));
+   Entry->lpSource = (LPCWSTR)(Entry + 1);
    wcscpy((LPWSTR)Entry->lpSource, lpSource);
    Entry->lpTarget = Entry->lpSource + dwSource;
    wcscpy((LPWSTR)Entry->lpTarget, lpTarget);
@@ -192,9 +208,10 @@
    while(RootHeader)
    {
        length += (wcslen(RootHeader->lpExeName) + 1) * sizeof(WCHAR);
+       RootHeader = RootHeader->Next;
    }
    if (length)
-       length++; // last entry entry is terminated with 2 zero bytes
+       length += sizeof(WCHAR); // last entry entry is terminated with 2 zero bytes
 
    return length;
 }
@@ -236,6 +253,7 @@
        Length += wcslen(CurEntry->lpSource);
        Length += wcslen(CurEntry->lpTarget);
        Length += 2; // zero byte and '='
+       CurEntry = CurEntry->Next;
    }
 
    if (Length)
@@ -374,7 +392,7 @@
 
     lpSource = (LPWSTR)((ULONG_PTR)Request + sizeof(CSR_API_MESSAGE));
     lpExeName = lpSource + Request->Data.GetConsoleAlias.SourceLength;
-    lpTarget = (LPWSTR)lpExeName + Request->Data.GetConsoleAlias.ExeLength;
+    lpTarget = Request->Data.GetConsoleAlias.TargetBuffer;
 
 
     DPRINT("CsrGetConsoleAlias entered lpExeName %p lpSource %p TargetBuffer %p
TargetBufferLength %u\n", 
@@ -404,23 +422,17 @@
     Length = (wcslen(Entry->lpTarget)+1) * sizeof(WCHAR);
     if (Length > Request->Data.GetConsoleAlias.TargetBufferLength)
     {
-        Request->Status = ERROR_INSUFFICIENT_BUFFER;
+        Request->Status = STATUS_BUFFER_TOO_SMALL;
         return Request->Status;      
     }
 
-#if 0
-    if (((PVOID)lpTarget < ProcessData->CsrSectionViewBase)
-      || (((ULONG_PTR)lpTarget + Request->Data.GetConsoleAlias.TargetBufferLength)
> ((ULONG_PTR)ProcessData->CsrSectionViewBase +
ProcessData->CsrSectionViewSize)))
+    if (!ValidateBuffer(ProcessData, lpTarget,
Request->Data.GetConsoleAlias.TargetBufferLength))
     {
         Request->Status = STATUS_ACCESS_VIOLATION;
-        DPRINT1("CsrGetConsoleAlias out of range lpTarget %p LowerViewBase %p
UpperViewBase %p Size %p\n", lpTarget, 
-            ProcessData->CsrSectionViewBase,
(ULONG_PTR)ProcessData->CsrSectionViewBase + ProcessData->CsrSectionViewSize,
ProcessData->CsrSectionViewSize);
-        return Request->Status;
-    }
-#endif
+        return Request->Status;
+    }
 
     wcscpy(lpTarget, Entry->lpTarget);
-    lpTarget[CSRSS_MAX_ALIAS_TARGET_LENGTH-1] = '\0';
     Request->Data.GetConsoleAlias.BytesWritten = Length;
     Request->Status = STATUS_SUCCESS;
     return Request->Status;
@@ -446,7 +458,15 @@
 
     if (IntGetAllConsoleAliasesLength(Header) >
Request->Data.GetAllConsoleAlias.AliasBufferLength)
     {
-        Request->Status = ERROR_INSUFFICIENT_BUFFER;
+        Request->Status = STATUS_BUFFER_OVERFLOW;
+        return Request->Status;
+    }
+
+    if (!ValidateBuffer(ProcessData,
+                        Request->Data.GetAllConsoleAlias.AliasBuffer,
+                        Request->Data.GetAllConsoleAlias.AliasBufferLength))
+    {
+        Request->Status = STATUS_ACCESS_VIOLATION;
         return Request->Status;
     }
 
@@ -495,7 +515,7 @@
     
     if (ExesLength > Request->Data.GetConsoleAliasesExes.Length)
     {
-        Request->Status = ERROR_INSUFFICIENT_BUFFER;
+        Request->Status = STATUS_BUFFER_OVERFLOW;
         return Request->Status;
     }
 
@@ -505,6 +525,14 @@
         return Request->Status;
     }
     
+    if (!ValidateBuffer(ProcessData,
+                        Request->Data.GetConsoleAliasesExes.ExeNames,
+                        Request->Data.GetConsoleAliasesExes.Length))
+    {
+        Request->Status = STATUS_ACCESS_VIOLATION;
+        return Request->Status;
+    }
+
     BytesWritten = IntGetConsoleAliasesExes(RootHeader, 
                                            
Request->Data.GetConsoleAliasesExes.ExeNames,
                                            
Request->Data.GetConsoleAliasesExes.Length);