[PATCH v7 0/3] MR10114: msi: Handle unusual installer files (padding, malformed strrefs)
This PR addresses two issues that I observed preventing the Native Instruments Kontakt Player MSI (and other MSI files from the same vendor) from successfully finishing: 1. Some installers contain padding for 4-byte alignment of table streams. * Proposed fix: We relax Wine's parsing to allow for such padding. 2. Some installers declare that they have 3-byte strrefs, but the data actually contains 2-byte strrefs. This looks like a malformed MSI to me, and Wine correctly rejects them currently. The regression tests in this PR indicate that Windows in CI (test-win10-21h2-\*) is reading garbage as well. However, NI ships these MSI files, which implies that somehow the installation successfully completes on Windows machines nevertheless. * Proposed fix: We explicitly detect and correct this particular mismatch at load time of the MSI, in order to be able to complete the installation under Wine as well. --- 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 user 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 logic bug in the hub or installation orchestration. 2. To observe/examine the underlying MSI files (including the fix presented here), you'll need to grab them after NA downloads and extracts them on your machine (or find manual/direct downloads on the NI website, where applicable) --- _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._ -- v7: msi/tests: Add regression tests for table stream padding and mismatched string refs. msi: Detect mismatched bytes_per_strref in table data. msi: Handle padding bytes in table streams. https://gitlab.winehq.org/wine/wine/-/merge_requests/10114
From: Theodoros Chatzigiannakis <tchatzigiannakis@gmail.com> --- dlls/msi/table.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/dlls/msi/table.c b/dlls/msi/table.c index 6031dd9c805..a99b00596b6 100644 --- a/dlls/msi/table.c +++ b/dlls/msi/table.c @@ -410,8 +410,14 @@ 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; + 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)) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10114
From: Theodoros Chatzigiannakis <tchatzigiannakis@gmail.com> --- dlls/msi/table.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/dlls/msi/table.c b/dlls/msi/table.c index a99b00596b6..897df069722 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,23 @@ static UINT read_table_from_storage( MSIDATABASE *db, MSITABLE *t, IStorage *stg TRACE("Read %d bytes\n", rawsize ); + if( rawsize % row_size ) + { + /* Check if the data was written with a different bytes_per_strref. This can happen + * when transforms change the string pool metadata without re-encoding all table streams. */ + if (bytes_per_strref == LONG_STR_BYTES) + { + 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) + { + WARN("Table %s: data uses %u-byte string refs, not %u\n", + debugstr_w(t->name), (UINT)sizeof(USHORT), bytes_per_strref); + bytes_per_strref = sizeof(USHORT); + row_size = alt_row_size; + } + } + } + if( rawsize % row_size ) { UINT padding = rawsize % row_size; @@ -440,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 | 279 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 279 insertions(+) diff --git a/dlls/msi/tests/db.c b/dlls/msi/tests/db.c index 56fff94c5df..79465b7bdeb 100644 --- a/dlls/msi/tests/db.c +++ b/dlls/msi/tests/db.c @@ -9365,6 +9365,283 @@ 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); + + query = "INSERT INTO `MOO` (`A`, `B`) VALUES (6, 'six')"; + 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 the modified MSI and verify the table is queryable 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); + r = MsiRecordGetInteger(hrec, 1); + ok(r == 1, "Expected 1, got %d\n", r); + MsiCloseHandle(hrec); + + MsiViewClose(hview); + MsiCloseHandle(hview); + MsiCloseHandle(hdb); + DeleteFileA(msifile); +} + START_TEST(db) { test_msidatabase(); @@ -9425,4 +9702,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)