[PATCH] [DbgHelp]: fix copy_line_W64_from_64 when A and W buffers are identical

Eric Pouech eric.pouech at gmail.com
Wed Aug 18 03:24:40 CDT 2021


As reported by Alistair, when doing Unicode conversion for FileName field
from IMAGEHLP_LINE64 into IMAGEHLP_LINE64W, both structures would refer
to the same buffer allocated by fetch_buffer.
So properly detect and handle this situation.

Signed-off-by: Eric Pouech <eric.pouech at gmail.com>

---
 dlls/dbghelp/symbol.c |   35 +++++++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/dlls/dbghelp/symbol.c b/dlls/dbghelp/symbol.c
index b2b4d578910..3b264eb9a52 100644
--- a/dlls/dbghelp/symbol.c
+++ b/dlls/dbghelp/symbol.c
@@ -1588,19 +1588,39 @@ static void copy_line_64_from_32(IMAGEHLP_LINE64* l64, const IMAGEHLP_LINE* l32)
 }
 
 /******************************************************************
- *		copy_line_W64_from_32 (internal)
+ *		copy_line_W64_from_64 (internal)
  *
+ * Note: upon return, l64w->FileName is always allocated by fetch_buffer().
+ * So, if input l64->FileName has *also* been allocated by fetch_buffer()
+ * before calling this function, l64->FileName will be overwritten
+ * (as it's the same buffer returned in l64w).
+ * In consequence, don't use the l64 structure *after* calling this function.
  */
-static void copy_line_W64_from_64(struct process* pcs, IMAGEHLP_LINEW64* l64w, const IMAGEHLP_LINE64* l64)
+static BOOL copy_line_W64_from_64(struct process* pcs, IMAGEHLP_LINEW64* l64w, const IMAGEHLP_LINE64* l64)
 {
-    unsigned len;
+    char* incoming = l64->FileName;
+    unsigned len = MultiByteToWideChar(CP_ACP, 0, l64->FileName, -1, NULL, 0);
+
+    /* detect when l64->FileName has been allocated by fetch_buffer()
+     * and make a copy before calling fetch_buffer() again
+     */
+    if (incoming >= (char*)pcs->buffer && incoming < (char*)pcs->buffer + pcs->buffer_size)
+    {
+        DWORD sz = strlen(incoming) + 1;
+        char* new = HeapAlloc(GetProcessHeap(), 0, sz);
+        if (!new)
+            return FALSE;
+        incoming = memcpy(new, incoming, sz);
+    }
 
     l64w->Key = l64->Key;
     l64w->LineNumber = l64->LineNumber;
-    len = MultiByteToWideChar(CP_ACP, 0, l64->FileName, -1, NULL, 0);
-    if ((l64w->FileName = fetch_buffer(pcs, len * sizeof(WCHAR))))
-        MultiByteToWideChar(CP_ACP, 0, l64->FileName, -1, l64w->FileName, len);
     l64w->Address = l64->Address;
+    if (!(l64w->FileName = fetch_buffer(pcs, len * sizeof(WCHAR))))
+        return FALSE;
+    MultiByteToWideChar(CP_ACP, 0, incoming, -1, l64w->FileName, len);
+    if (incoming != l64->FileName) HeapFree(GetProcessHeap(), 0, incoming);
+    return TRUE;
 }
 
 /******************************************************************
@@ -1671,8 +1691,7 @@ BOOL WINAPI SymGetLineFromAddrW64(HANDLE hProcess, DWORD64 dwAddr,
     il64.SizeOfStruct = sizeof(il64);
     if (!SymGetLineFromAddr64(hProcess, dwAddr, pdwDisplacement, &il64))
         return FALSE;
-    copy_line_W64_from_64(process_find_by_handle(hProcess), Line, &il64);
-    return TRUE;
+    return copy_line_W64_from_64(process_find_by_handle(hProcess), Line, &il64);
 }
 
 /******************************************************************




More information about the wine-devel mailing list