[PATCH 0/3] MR10114: msi: handle unusual installer files (padding, malformed string refs)
This PR addresses two issues that I observed preventing the Native Instruments Kontakt Player MSI (and presumably other MSI files from the same vendor): 1. Some installers contain padding for 4-byte alignment of table streams. We relax Wine's parsing to allow for such padding. 2. Some installers declare that they have 3-byte string refs, but the data actually contains 2-byte string refs. This looks like a malformed MSI to me (a result of a misapplied transform when creating the installer perhaps). However, Windows presumably(\*) accepts this inconsistency, so we try to be forgiving here -- even though the current behavior is not an actual bug in Wine (as far as I can tell). (\*) I have not tried to install NI software in Windows for years. But Windows is a main platform for NI, so if Windows didn't accept these malformed files, then NI (and other vendors) would presumably have caught this issue in their installers and addressed it already. --- The above are technically separate issues, but they affect the same software in similar ways, so I opted for one PR. Please me know if I should split them. --- Quick repro hint: Native Instruments offers a program called Native Access (which acts as an installation hub on the user's machine). On the user's request, NA downloads and launches the affected MSI files in the background. If you try to repro, a couple of things to watch out for: 1. NA seems to always report a successful installation, even when the underlying MSI fails to complete (and therefore nothing is installed). This PR does nothing to address this false positive, as it is most likely a NA logic bug (i.e. not checking the installer's status or something similar). 2. To observe/examine the underlying MSI files (including the fix presented here), you'll need to grab them after NA downloads them on your machine (or find manual/direct downloads on the NI website, if any) --- _Disclaimers for reviewers:_ * _AI was used while tracking down the issues and coming up with fixes._ * _I am not affiliated with Native Instruments, I'm just a user._ -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10114
From: Theodoros Chatzigiannakis <tchatzigiannakis@gmail.com> --- dlls/msi/table.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/dlls/msi/table.c b/dlls/msi/table.c index 6031dd9c805..44de70841ea 100644 --- a/dlls/msi/table.c +++ b/dlls/msi/table.c @@ -410,11 +410,19 @@ static UINT read_table_from_storage( MSIDATABASE *db, MSITABLE *t, IStorage *stg if( rawsize % row_size ) { - WARN("Table size is invalid %d/%d\n", rawsize, row_size ); - goto err; + /* MSI files may have streams padded to 4-byte alignment */ + UINT padding = rawsize % row_size; + if (padding < 4) + rawsize -= padding; + else + { + WARN("Table size is invalid %d/%d\n", rawsize, row_size ); + goto err; + } } - if ((t->row_count = rawsize / row_size)) + t->row_count = rawsize / row_size; + if (t->row_count) { if (!(t->data = calloc( t->row_count, sizeof(*t->data) ))) goto err; if (!(t->data_persistent = calloc( t->row_count, sizeof(BOOL) ))) goto err; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10114
From: Theodoros Chatzigiannakis <tchatzigiannakis@gmail.com> --- dlls/msi/table.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/dlls/msi/table.c b/dlls/msi/table.c index 44de70841ea..ab3bb5dab2f 100644 --- a/dlls/msi/table.c +++ b/dlls/msi/table.c @@ -394,11 +394,12 @@ static UINT table_get_row_size( MSIDATABASE *db, const struct column_info *cols, static UINT read_table_from_storage( MSIDATABASE *db, MSITABLE *t, IStorage *stg ) { BYTE *rawdata = NULL; - UINT rawsize = 0, i, j, row_size, row_size_mem; + UINT rawsize = 0, i, j, row_size, row_size_mem, bytes_per_strref; TRACE("%s\n",debugstr_w(t->name)); - row_size = table_get_row_size( db, t->colinfo, t->col_count, db->bytes_per_strref ); + bytes_per_strref = db->bytes_per_strref; + row_size = table_get_row_size( db, t->colinfo, t->col_count, bytes_per_strref ); row_size_mem = table_get_row_size( db, t->colinfo, t->col_count, LONG_STR_BYTES ); /* if we can't read the table, just assume that it's empty */ @@ -408,6 +409,21 @@ static UINT read_table_from_storage( MSIDATABASE *db, MSITABLE *t, IStorage *stg TRACE("Read %d bytes\n", rawsize ); + /* Check if table data was written with a different bytes_per_strref than the database. + * Some installers create MSI files with mismatched string reference sizes. + * Skip system tables (_Columns, _Tables) as they may be updated during transforms. */ + if (bytes_per_strref == LONG_STR_BYTES && t->name[0] != '_') + { + UINT alt_row_size = table_get_row_size( db, t->colinfo, t->col_count, sizeof(USHORT) ); + if (alt_row_size != row_size && rawsize % alt_row_size == 0 && rawsize / alt_row_size > rawsize / row_size) + { + WARN("Table %s: detected 2-byte string refs (rawsize %u fits %u rows vs %u with db bytes_per_strref)\n", + debugstr_w(t->name), rawsize, rawsize / alt_row_size, rawsize / row_size); + bytes_per_strref = sizeof(USHORT); + row_size = alt_row_size; + } + } + if( rawsize % row_size ) { /* MSI files may have streams padded to 4-byte alignment */ @@ -442,7 +458,7 @@ static UINT read_table_from_storage( MSIDATABASE *db, MSITABLE *t, IStorage *stg for (j = 0; j < t->col_count; j++) { UINT m = bytes_per_column( db, &t->colinfo[j], LONG_STR_BYTES ); - UINT n = bytes_per_column( db, &t->colinfo[j], db->bytes_per_strref ); + UINT n = bytes_per_column( db, &t->colinfo[j], bytes_per_strref ); UINT k; if ( n != 2 && n != 3 && n != 4 ) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10114
From: Theodoros Chatzigiannakis <tchatzigiannakis@gmail.com> --- dlls/msi/tests/db.c | 297 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 297 insertions(+) diff --git a/dlls/msi/tests/db.c b/dlls/msi/tests/db.c index 56fff94c5df..26b5966bf43 100644 --- a/dlls/msi/tests/db.c +++ b/dlls/msi/tests/db.c @@ -9365,6 +9365,301 @@ static void test_viewfetch_wraparound(void) DeleteFileA(msifile); } +static void write_raw_stream(IStorage *stg, const WCHAR *name, const void *data, DWORD size) +{ + IStream *stm; + DWORD count; + HRESULT hr; + + hr = IStorage_CreateStream(stg, name, STGM_WRITE | STGM_SHARE_EXCLUSIVE, 0, 0, &stm); + ok(hr == S_OK, "Expected S_OK, got %#lx\n", hr); + hr = IStream_Write(stm, data, size, &count); + ok(hr == S_OK, "Expected S_OK, got %#lx\n", hr); + IStream_Release(stm); +} + +static void test_table_stream_padding(void) +{ + static const WCHAR moo_encoded[] = {0x4840, 0x3e16, 0x4818, 0}; /* encoded "MOO" */ + MSIHANDLE hdb, hview, hrec; + IStorage *stg; + IStream *stm; + WCHAR nameW[MAX_PATH]; + HRESULT hr; + BYTE moo_data[64]; + BYTE padded[64]; + DWORD moo_size; + const char *query; + UINT r; + + DeleteFileA(msifile); + + + r = MsiOpenDatabaseW(msifileW, MSIDBOPEN_CREATE, &hdb); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + + query = "CREATE TABLE `MOO` (`A` LONG PRIMARY KEY `A`)"; + r = run_query(hdb, 0, query); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + + query = "INSERT INTO `MOO` (`A`) VALUES (10)"; + r = run_query(hdb, 0, query); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + + query = "INSERT INTO `MOO` (`A`) VALUES (20)"; + r = run_query(hdb, 0, query); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + + r = MsiDatabaseCommit(hdb); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + MsiCloseHandle(hdb); + + + MultiByteToWideChar(CP_ACP, 0, msifile, -1, nameW, MAX_PATH); + hr = StgOpenStorage(nameW, NULL, STGM_DIRECT | STGM_READWRITE | STGM_SHARE_EXCLUSIVE, + NULL, 0, &stg); + ok(hr == S_OK, "Expected S_OK, got %#lx\n", hr); + + hr = IStorage_OpenStream(stg, moo_encoded, NULL, STGM_READ | STGM_SHARE_EXCLUSIVE, 0, &stm); + ok(hr == S_OK, "Expected S_OK, got %#lx\n", hr); + hr = IStream_Read(stm, moo_data, sizeof(moo_data), &moo_size); + ok(hr == S_OK, "Expected S_OK, got %#lx\n", hr); + ok(moo_size == 8, "Expected 8, got %lu\n", moo_size); + IStream_Release(stm); + + + memcpy(padded, moo_data, moo_size); + padded[moo_size] = 0; + padded[moo_size + 1] = 0; + + + hr = IStorage_DestroyElement(stg, moo_encoded); + ok(hr == S_OK, "Expected S_OK, got %#lx\n", hr); + + hr = IStorage_CreateStream(stg, moo_encoded, STGM_WRITE | STGM_SHARE_EXCLUSIVE, 0, 0, &stm); + ok(hr == S_OK, "Expected S_OK, got %#lx\n", hr); + hr = IStream_Write(stm, padded, moo_size + 2, &moo_size); + ok(hr == S_OK, "Expected S_OK, got %#lx\n", hr); + IStream_Release(stm); + + IStorage_Release(stg); + + + r = MsiOpenDatabaseW(msifileW, MSIDBOPEN_READONLY, &hdb); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + + query = "SELECT `A` FROM `MOO`"; + r = MsiDatabaseOpenViewA(hdb, query, &hview); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + + r = MsiViewExecute(hview, 0); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + + r = MsiViewFetch(hview, &hrec); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + check_record(hrec, 1, "10"); + MsiCloseHandle(hrec); + + r = MsiViewFetch(hview, &hrec); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + check_record(hrec, 1, "20"); + MsiCloseHandle(hrec); + + r = MsiViewFetch(hview, &hrec); + ok(r == ERROR_NO_MORE_ITEMS, "Expected ERROR_NO_MORE_ITEMS, got %d\n", r); + + MsiViewClose(hview); + MsiCloseHandle(hview); + MsiCloseHandle(hdb); + DeleteFileA(msifile); +} + +/* Convert column-major table data from 2-byte to 3-byte string references. Returns the new size. */ +static DWORD convert_to_3byte_strref(const BYTE *old_data, DWORD old_size, + BYTE *new_data, const int *col_types, UINT col_count) +{ + UINT old_row_size = col_count * 2; + UINT row_count = old_size / old_row_size; + DWORD new_size = 0; + UINT old_ofs = 0; + UINT j, i; + + for (j = 0; j < col_count; j++) + { + for (i = 0; i < row_count; i++) + { + new_data[new_size++] = old_data[old_ofs++]; + new_data[new_size++] = old_data[old_ofs++]; + if (!col_types[j]) + new_data[new_size++] = 0; + } + } + return new_size; +} + +static void test_table_mismatched_strref(void) +{ + static const CLSID CLSID_MsiDatabase = + { 0xc1084, 0, 0, {0xc0, 0, 0, 0, 0, 0, 0, 0x46} }; + static const WCHAR stringpool_enc[] = {0x4840, 0x3f3f, 0x4577, 0x446c, 0x3e6a, 0x44b2, 0x482f, 0}; + static const WCHAR tables_enc[] = {0x4840, 0x3f7f, 0x4164, 0x422f, 0x4836, 0}; + static const WCHAR columns_enc[] = {0x4840, 0x3b3f, 0x43f2, 0x4438, 0x45b1, 0}; + static const int tables_col_types[] = {0}; + static const int columns_col_types[] = {0, 1, 0, 1}; + + MSIHANDLE hdb, hview, hrec; + IStorage *src, *dst; + IEnumSTATSTG *enumstg; + STATSTG stat; + IStream *stm; + WCHAR nameW[MAX_PATH], tmpW[MAX_PATH]; + HRESULT hr; + BYTE buf[4096], converted[4096]; + DWORD size, new_size; + const char *query; + UINT r; + + DeleteFileA(msifile); + + /* Create a valid MSI database with 2-byte string refs */ + r = MsiOpenDatabaseW(msifileW, MSIDBOPEN_CREATE, &hdb); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + + query = "CREATE TABLE `MOO` (`A` SHORT NOT NULL, `B` CHAR(72) PRIMARY KEY `A`)"; + r = run_query(hdb, 0, query); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + + query = "INSERT INTO `MOO` (`A`, `B`) VALUES (1, 'one')"; + r = run_query(hdb, 0, query); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + + query = "INSERT INTO `MOO` (`A`, `B`) VALUES (2, 'two')"; + r = run_query(hdb, 0, query); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + + query = "INSERT INTO `MOO` (`A`, `B`) VALUES (3, 'three')"; + r = run_query(hdb, 0, query); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + + query = "INSERT INTO `MOO` (`A`, `B`) VALUES (4, 'four')"; + r = run_query(hdb, 0, query); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + + query = "INSERT INTO `MOO` (`A`, `B`) VALUES (5, 'five')"; + r = run_query(hdb, 0, query); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + + r = MsiDatabaseCommit(hdb); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + MsiCloseHandle(hdb); + + /* Rebuild the MSI with modifications: + * - Set 0x8000 flag in _StringPool (declares 3-byte string refs) + * - Convert _Tables and _Columns to 3-byte string refs + * - Leave MOO stream unchanged (2-byte string refs) to simulate mismatch */ + MultiByteToWideChar(CP_ACP, 0, msifile, -1, nameW, MAX_PATH); + hr = StgOpenStorage(nameW, NULL, STGM_READ | STGM_SHARE_DENY_WRITE, NULL, 0, &src); + ok(hr == S_OK, "Expected S_OK, got %#lx\n", hr); + + MultiByteToWideChar(CP_ACP, 0, "msitest2.msi", -1, tmpW, MAX_PATH); + hr = StgCreateDocfile(tmpW, STGM_CREATE | STGM_READWRITE | STGM_DIRECT | STGM_SHARE_EXCLUSIVE, + 0, &dst); + ok(hr == S_OK, "Expected S_OK, got %#lx\n", hr); + + hr = IStorage_SetClass(dst, &CLSID_MsiDatabase); + ok(hr == S_OK, "Expected S_OK, got %#lx\n", hr); + + hr = IStorage_EnumElements(src, 0, NULL, 0, &enumstg); + ok(hr == S_OK, "Expected S_OK, got %#lx\n", hr); + + while (IEnumSTATSTG_Next(enumstg, 1, &stat, NULL) == S_OK) + { + if (stat.type == STGTY_STREAM) + { + hr = IStorage_OpenStream(src, stat.pwcsName, NULL, + STGM_READ | STGM_SHARE_EXCLUSIVE, 0, &stm); + ok(hr == S_OK, "Expected S_OK, got %#lx\n", hr); + hr = IStream_Read(stm, buf, sizeof(buf), &size); + ok(hr == S_OK, "Expected S_OK, got %#lx\n", hr); + IStream_Release(stm); + + if (!lstrcmpW(stat.pwcsName, stringpool_enc)) + { + buf[3] |= 0x80; + write_raw_stream(dst, stat.pwcsName, buf, size); + } + else if (!lstrcmpW(stat.pwcsName, tables_enc)) + { + new_size = convert_to_3byte_strref(buf, size, converted, tables_col_types, 1); + write_raw_stream(dst, stat.pwcsName, converted, new_size); + } + else if (!lstrcmpW(stat.pwcsName, columns_enc)) + { + new_size = convert_to_3byte_strref(buf, size, converted, columns_col_types, 4); + write_raw_stream(dst, stat.pwcsName, converted, new_size); + } + else + { + write_raw_stream(dst, stat.pwcsName, buf, size); + } + } + CoTaskMemFree(stat.pwcsName); + } + + IEnumSTATSTG_Release(enumstg); + IStorage_Release(src); + IStorage_Release(dst); + + r = DeleteFileA(msifile); + ok(r, "DeleteFileA failed: %lu\n", GetLastError()); + r = MoveFileA("msitest2.msi", msifile); + ok(r, "MoveFileA failed: %lu\n", GetLastError()); + + /* Open as MSI database and verify rows read correctly despite mismatched strref */ + r = MsiOpenDatabaseW(msifileW, MSIDBOPEN_READONLY, &hdb); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + + query = "SELECT `A`, `B` FROM `MOO`"; + r = MsiDatabaseOpenViewA(hdb, query, &hview); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + + r = MsiViewExecute(hview, 0); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + + r = MsiViewFetch(hview, &hrec); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + check_record(hrec, 2, "1", "one"); + MsiCloseHandle(hrec); + + r = MsiViewFetch(hview, &hrec); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + check_record(hrec, 2, "2", "two"); + MsiCloseHandle(hrec); + + r = MsiViewFetch(hview, &hrec); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + check_record(hrec, 2, "3", "three"); + MsiCloseHandle(hrec); + + r = MsiViewFetch(hview, &hrec); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + check_record(hrec, 2, "4", "four"); + MsiCloseHandle(hrec); + + r = MsiViewFetch(hview, &hrec); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r); + check_record(hrec, 2, "5", "five"); + MsiCloseHandle(hrec); + + r = MsiViewFetch(hview, &hrec); + ok(r == ERROR_NO_MORE_ITEMS, "Expected ERROR_NO_MORE_ITEMS, got %d\n", r); + + MsiViewClose(hview); + MsiCloseHandle(hview); + MsiCloseHandle(hdb); + DeleteFileA(msifile); +} + START_TEST(db) { test_msidatabase(); @@ -9425,4 +9720,6 @@ START_TEST(db) test_viewmodify_insert(); test_view_get_error(); test_viewfetch_wraparound(); + test_table_stream_padding(); + test_table_mismatched_strref(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10114
participants (2)
-
Theodoros Chatzigiannakis -
Theodoros Chatzigiannakis (ï¼ TChatzigiannakis)