[PATCH 0/1] MR5452: winegstreamer: Try to handle broken IStream_Stat implementation in WM reader OpenStream().

[PATCH 1/1] winegstreamer: Try to handle broken IStream_Stat implementation in WM reader OpenStream().

[Paul Gofman]

From: Paul Gofman

---
 dlls/winegstreamer/wm_reader.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/dlls/winegstreamer/wm_reader.c b/dlls/winegstreamer/wm_reader.c
index 18910f87d40..c96d053ccae 100644
--- a/dlls/winegstreamer/wm_reader.c
+++ b/dlls/winegstreamer/wm_reader.c
@@ -2224,18 +2224,36 @@ static HRESULT WINAPI reader_Open(IWMSyncReader2 *iface, const WCHAR *filename)

static HRESULT WINAPI reader_OpenStream(IWMSyncReader2 *iface, IStream *stream)
 {
+    static const ULONG64 canary_size = 0xdeadbeeffeedcafe;
     struct wm_reader *reader = impl_from_IWMSyncReader2(iface);
     STATSTG stat;
     HRESULT hr;

TRACE("reader %p, stream %p.\n", reader, stream);

+    stat.cbSize.QuadPart = canary_size;
     if (FAILED(hr = IStream_Stat(stream, &stat, STATFLAG_NONAME)))
     {
         ERR("Failed to stat stream, hr %#lx.\n", hr);
         return hr;
     }

+    if (stat.cbSize.QuadPart == canary_size)
+    {
+        /* Call of Juarez: Gunslinger implements IStream_Stat as an empty function returning S_OK, leaving
+         * the output stat unchanged. Windows doesn't call IStream_Seek(_SEEK_END) and probably validates
+         * the size against WMV file headers so the bigger cbSize doesn't change anything.
+         * Such streams work as soon as the uninitialized cbSize is big enough which is usually the case
+         * (if that is not the case Windows will favour shorter cbSize). */
+        static const LARGE_INTEGER zero = { 0 };
+        ULARGE_INTEGER pos = { .QuadPart = canary_size };
+
+        if (SUCCEEDED(hr = IStream_Seek(stream, zero, STREAM_SEEK_END, &pos)))
+            IStream_Seek(stream, zero, STREAM_SEEK_SET, NULL);
+        stat.cbSize.QuadPart = pos.QuadPart == canary_size ? 0 : pos.QuadPart;
+        ERR("IStream_Stat did not fill the stream size, size from _Seek %I64u.\n", stat.cbSize.QuadPart);
+    }
+
     EnterCriticalSection(&reader->cs);

if (reader->wg_parser)

[Paul Gofman (＠gofman)]

Call of Juarez: Gunslinger's IStream::Stat returns S_OK but nothing is set in the output structure. So we end up with uninitialized stream size value. That is usually a big value, and then attempt to read with IStream::Read() with huge current position results in a crash in memcpy called from game's IStream::Read implementation.

I reproduced what the game is doing on Windows (attaching an ad-hoc test which is probably too weird to be included). In the essence of that, ending up with larger size from ::Stat (either setting it explicitly or having as random) doesn't change anything at all on Windows. The smaller size is favoured. So it seems like Windows gets the size from WMV headers and favours that together with reported stream size. Doing the same in Wine looks problematic. A straightforward trivial way, obtaining the size from stream data, means parsing a patented format which we probably want to avoid in Wine code. The gstreamer part relies on the stream size to be known.

[test.patch](/uploads/5e7d0fd8df971ce3dfb13027433a3aae/test.patch)

[Nikolay Sivov (＠nsivov)]

On Wed Apr  3 22:19:48 2024 +0000, Paul Gofman wrote:
> Call of Juarez: Gunslinger's IStream::Stat returns S_OK but nothing is
> set in the output structure. So we end up with uninitialized stream size
> value. That is usually a big value, and then attempt to read with
> IStream::Read() with huge current position results in a crash in memcpy
> called from game's IStream::Read implementation.
> I reproduced what the game is doing on Windows (attaching an ad-hoc test
> which is probably too weird to be included). In the essence of that,
> ending up with larger size from ::Stat (either setting it explicitly or
> having as random) doesn't change anything at all on Windows. The smaller
> size is favoured. So it seems like Windows gets the size from WMV
> headers and favours that together with reported stream size. Doing the
> same in Wine looks problematic. A straightforward trivial way, obtaining
> the size from stream data, means parsing a patented format which we
> probably want to avoid in Wine code. The gstreamer part relies on the
> stream size to be known.
> [test.patch](/uploads/5e7d0fd8df971ce3dfb13027433a3aae/test.patch)

Is Stat() used at all on Windows?

[Paul Gofman (＠gofman)]

On Wed Apr  3 22:11:26 2024 +0000, Nikolay Sivov wrote:
> Is Stat() used at all on Windows?

Yes, it is covered by existing tests, as well as my ad-hoc addition. ::Seek(_SEEK_END) is not used.

[Elizabeth Figura (＠zfigura)]

This is certainly ugly, but I don't see a better way.

[PATCH 0/1] MR5452: winegstreamer: Try to handle broken IStream_Stat implementation in WM reader OpenStream(). - approved

[Elizabeth Figura (＠zfigura)]

This merge request was approved by Elizabeth Figura.