[PATCH 0/1] MR6013: msi: Avoid infinite recursion while processing the DrLocator table.

[Hans Leidekker (＠hans)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56938

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6013

[PATCH 1/1] msi: Avoid infinite recursion while processing the DrLocator table.

[Hans Leidekker]

From: Hans Leidekker <hans(a)codeweavers.com>

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56938
---
 dlls/msi/appsearch.c | 134 ++++++++++++++++++++++++++++---------------
 dlls/msi/msipriv.h   |  11 ++++
 dlls/msi/package.c   |   1 +
 3 files changed, 100 insertions(+), 46 deletions(-)

diff --git a/dlls/msi/appsearch.c b/dlls/msi/appsearch.c
index 381e74317fe..070f31c5227 100644
--- a/dlls/msi/appsearch.c
+++ b/dlls/msi/appsearch.c
@@ -940,80 +940,122 @@ static UINT search_directory( MSIPACKAGE *package, struct signature *sig, const
     return rc;
 }
 
+static UINT load_drlocator( MSIRECORD *row, void *param )
+{
+    MSIPACKAGE *package = param;
+    MSIDRLOCATOR *locator;
+
+    if (!(locator = calloc( 1, sizeof(MSIDRLOCATOR) ))) return ERROR_FUNCTION_FAILED;
+
+    locator->Signature = msi_dup_record_field( row, 1 );
+
+    TRACE("Loading DrLocator %s\n", debugstr_w(locator->Signature));
+
+    locator->Parent     = msi_dup_record_field( row, 2 );
+    locator->Path       = msi_dup_record_field( row, 3 );
+    locator->Depth      = MSI_RecordGetInteger( row,4 );
+    list_add_tail( &package->drlocators, &locator->entry );
+    return ERROR_SUCCESS;
+}
+
+static UINT load_all_drlocators( MSIPACKAGE *package )
+{
+    MSIQUERY *view;
+    UINT r;
+
+    r = MSI_DatabaseOpenViewW( package->db, L"SELECT * FROM `DrLocator`", &view );
+    if (r != ERROR_SUCCESS)
+        return ERROR_SUCCESS;
+
+    r = MSI_IterateRecords( view, NULL, load_drlocator, package );
+    msiobj_release( &view->hdr );
+    return r;
+}
+
+static void free_drlocators( MSIPACKAGE *package )
+{
+    struct list *item, *cursor;
+
+    LIST_FOR_EACH_SAFE( item, cursor, &package->drlocators )
+    {
+        MSIDRLOCATOR *locator = LIST_ENTRY( item, MSIDRLOCATOR, entry );
+
+        list_remove( &locator->entry );
+        free( locator->Signature );
+        free( locator->Parent );
+        free( locator->Path );
+        free( locator );
+    }
+}
+
 static UINT search_sig_name( MSIPACKAGE *, const WCHAR *, struct signature *, WCHAR ** );
 
-static UINT search_dr( MSIPACKAGE *package, WCHAR **appValue, struct signature *sig )
+static UINT search_dr( MSIPACKAGE *package, WCHAR **app_value, struct signature *sig )
 {
-    LPWSTR parent = NULL;
-    LPCWSTR parentName;
-    WCHAR path[MAX_PATH];
-    WCHAR expanded[MAX_PATH];
-    MSIRECORD *row;
-    int depth;
-    DWORD sz, attr;
+    WCHAR path[MAX_PATH], expanded[MAX_PATH], *parent = NULL;
+    MSIDRLOCATOR *locator = NULL;
+    DWORD attr;
     UINT rc;
 
     TRACE("%s\n", debugstr_w(sig->Name));
 
-    *appValue = NULL;
+    if ((rc = load_all_drlocators( package ))) return rc;
 
-    row = MSI_QueryGetRecord( package->db, L"SELECT * FROM `DrLocator` WHERE `Signature_` = '%s'", sig->Name );
-    if (!row)
+    *app_value = NULL;
+
+    LIST_FOR_EACH_ENTRY( locator, &package->drlocators, MSIDRLOCATOR, entry )
     {
-        TRACE("failed to query DrLocator for %s\n", debugstr_w(sig->Name));
-        return ERROR_SUCCESS;
+        if (!wcscmp( sig->Name, locator->Signature )) break;
     }
 
-    /* check whether parent is set */
-    parentName = MSI_RecordGetString(row, 2);
-    if (parentName)
+    if (!locator)
     {
-        struct signature parentSig;
-
-        search_sig_name( package, parentName, &parentSig, &parent );
-        free_signature( &parentSig );
-        if (!parent)
-        {
-            msiobj_release(&row->hdr);
-            return ERROR_SUCCESS;
-        }
+        TRACE("failed to find DrLocator for %s\n", debugstr_w(sig->Name));
+        goto done;
+    }
+    if (locator->Seen)
+    {
+        TRACE("DrLocator %s already seen\n", debugstr_w(sig->Name));
+        goto done;
     }
+    locator->Seen = TRUE;
 
-    sz = MAX_PATH;
-    MSI_RecordGetStringW(row, 3, path, &sz);
+    if (locator->Parent)
+    {
+        struct signature parent_sig;
 
-    if (MSI_RecordIsNull(row,4))
-        depth = 0;
-    else
-        depth = MSI_RecordGetInteger(row,4);
+        search_sig_name( package, locator->Parent, &parent_sig, &parent );
+        free_signature( &parent_sig );
+        if (!parent) return ERROR_SUCCESS;
+    }
 
-    if (sz)
-        expand_any_path( package, path, expanded, MAX_PATH );
+    if (locator->Path)
+        expand_any_path( package, locator->Path, expanded, MAX_PATH );
     else
-        lstrcpyW(expanded, path);
+        expanded[0] = 0;
 
     if (parent)
     {
         attr = msi_get_file_attributes( package, parent );
-        if (attr != INVALID_FILE_ATTRIBUTES &&
-            !(attr & FILE_ATTRIBUTE_DIRECTORY))
+        if (attr != INVALID_FILE_ATTRIBUTES && !(attr & FILE_ATTRIBUTE_DIRECTORY))
         {
-            PathRemoveFileSpecW(parent);
-            PathAddBackslashW(parent);
+            PathRemoveFileSpecW( parent );
+            PathAddBackslashW( parent );
         }
 
-        lstrcpyW(path, parent);
-        lstrcatW(path, expanded);
+        wcscpy( path, parent );
+        wcscat( path, expanded );
     }
-    else if (sz) lstrcpyW(path, expanded);
+    else wcscpy( path, expanded );
 
-    PathAddBackslashW(path);
+    PathAddBackslashW( path );
 
-    rc = search_directory( package, sig, path, depth, appValue );
+    rc = search_directory( package, sig, path, locator->Depth, app_value );
 
-    free(parent);
-    msiobj_release(&row->hdr);
-    TRACE("returning %d\n", rc);
+done:
+    free_drlocators( package );
+    free( parent );
+    TRACE("returning %u\n", rc);
     return rc;
 }
 
diff --git a/dlls/msi/msipriv.h b/dlls/msi/msipriv.h
index b7bc1e7b067..e5612331d60 100644
--- a/dlls/msi/msipriv.h
+++ b/dlls/msi/msipriv.h
@@ -406,6 +406,7 @@ typedef struct tagMSIPACKAGE
     struct list folders;
     struct list binaries;
     struct list cabinet_streams;
+    struct list drlocators;
     LPWSTR ActionFormat;
     LPWSTR LastAction;
     LPWSTR LastActionTemplate;
@@ -715,6 +716,16 @@ struct tagMSIMIME
     MSICLASS *Class;
 };
 
+typedef struct
+{
+    struct list entry;
+    WCHAR *Signature;
+    WCHAR *Parent;
+    WCHAR *Path;
+    int Depth;
+    BOOL Seen;
+} MSIDRLOCATOR;
+
 #define SEQUENCE_UI       0x1
 #define SEQUENCE_EXEC     0x2
 
diff --git a/dlls/msi/package.c b/dlls/msi/package.c
index 8c2168d9226..fb5463823e3 100644
--- a/dlls/msi/package.c
+++ b/dlls/msi/package.c
@@ -933,6 +933,7 @@ static MSIPACKAGE *alloc_package( void )
         list_init( &package->patches );
         list_init( &package->binaries );
         list_init( &package->cabinet_streams );
+        list_init( &package->drlocators );
     }
 
     return package;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/6013

[PATCH 1/1] msi: Avoid infinite recursion while processing the DrLocator table.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=146874

Your paranoid android.


=== debian11b (64 bit WoW report) ===

user32:
win.c:4037: Test failed: Expected active window 00000000028B0134, got 0000000001A00172.
win.c:4038: Test failed: Expected focus window 00000000028B0134, got 0000000001A00172.