[PATCH 2/2] ddraw: Don't read past the end of the executebuffer.

Stefan Dösinger stefan at codeweavers.com
Thu Apr 5 05:59:14 CDT 2018


This fixes bug 44864. The game passes an absurdly high vertex count to
SetExecuteData.

Signed-off-by: Stefan Dösinger <stefan at codeweavers.com>

---

It is possible that the vertex count in SetExecuteData should be ignored
entirely and only the count in the D3DOP_PROCESSVERTICES instructions
matters. In this case I'd handle this by placing the entire execute
buffer in a user memory wined3d buffer. I think that's overkill
considering how few d3d1-games exist and that we don't have a bug that
indicates we're storing too few vertices in the input buffer.
---
 dlls/ddraw/executebuffer.c |  10 ++--
 dlls/ddraw/tests/ddraw1.c  | 127 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 134 insertions(+), 3 deletions(-)

diff --git a/dlls/ddraw/executebuffer.c b/dlls/ddraw/executebuffer.c
index 89d9ed3a26..b20b9eda71 100644
--- a/dlls/ddraw/executebuffer.c
+++ b/dlls/ddraw/executebuffer.c
@@ -607,6 +607,7 @@ static HRESULT WINAPI d3d_execute_buffer_SetExecuteData(IDirect3DExecuteBuffer *
     struct wined3d_map_desc map_desc;
     struct wined3d_box box = {0};
     HRESULT hr;
+    DWORD buf_size = buffer->desc.dwBufferSize, copy_size;
 
     TRACE("iface %p, data %p.\n", iface, data);
 
@@ -662,7 +663,7 @@ static HRESULT WINAPI d3d_execute_buffer_SetExecuteData(IDirect3DExecuteBuffer *
         buffer->src_vertex_pos = 0;
     }
 
-    if (data->dwVertexCount)
+    if (data->dwVertexCount && (!buf_size || data->dwVertexOffset < buf_size))
     {
         box.left = buffer->src_vertex_pos * sizeof(D3DVERTEX);
         box.right = box.left + data->dwVertexCount * sizeof(D3DVERTEX);
@@ -670,8 +671,11 @@ static HRESULT WINAPI d3d_execute_buffer_SetExecuteData(IDirect3DExecuteBuffer *
                 0, &map_desc, &box, WINED3D_MAP_WRITE)))
             return hr;
 
-        memcpy(map_desc.data, ((BYTE *)buffer->desc.lpData) + data->dwVertexOffset,
-                data->dwVertexCount * sizeof(D3DVERTEX));
+        copy_size = data->dwVertexCount * sizeof(D3DVERTEX);
+        if (buf_size)
+            copy_size = min(copy_size, buf_size - data->dwVertexOffset);
+
+        memcpy(map_desc.data, ((BYTE *)buffer->desc.lpData) + data->dwVertexOffset, copy_size);
 
         wined3d_resource_unmap(wined3d_buffer_get_resource(buffer->src_vertex_buffer), 0);
     }
diff --git a/dlls/ddraw/tests/ddraw1.c b/dlls/ddraw/tests/ddraw1.c
index 626b07bace..c63ddee5e9 100644
--- a/dlls/ddraw/tests/ddraw1.c
+++ b/dlls/ddraw/tests/ddraw1.c
@@ -11247,6 +11247,132 @@ static void test_enum_surfaces(void)
     IDirectDraw_Release(ddraw);
 }
 
+static void test_execute_data(void)
+{
+    IDirect3DExecuteBuffer *execute_buffer;
+    D3DEXECUTEBUFFERDESC exec_desc;
+    IDirect3DDevice *device;
+    IDirectDraw *ddraw;
+    HWND window;
+    HRESULT hr;
+    D3DEXECUTEDATA exec_data;
+
+    window = create_window();
+    ddraw = create_ddraw();
+    ok(!!ddraw, "Failed to create a ddraw object.\n");
+    if (!(device = create_device(ddraw, window, DDSCL_NORMAL)))
+    {
+        skip("Failed to create a 3D device, skipping test.\n");
+        IDirectDraw_Release(ddraw);
+        DestroyWindow(window);
+        return;
+    }
+
+    memset(&exec_desc, 0, sizeof(exec_desc));
+    exec_desc.dwSize = sizeof(exec_desc);
+    exec_desc.dwFlags = D3DDEB_BUFSIZE | D3DDEB_CAPS;
+    exec_desc.dwBufferSize = 1024;
+    exec_desc.dwCaps = D3DDEBCAPS_SYSTEMMEMORY;
+
+    hr = IDirect3DDevice_CreateExecuteBuffer(device, &exec_desc, &execute_buffer, NULL);
+    ok(SUCCEEDED(hr), "Failed to create execute buffer, hr %#x.\n", hr);
+
+    memset(&exec_data, 0, sizeof(exec_data));
+
+    /* Success case. */
+    exec_data.dwSize = sizeof(exec_data);
+    exec_data.dwVertexCount = 3;
+    exec_data.dwInstructionOffset = 3 * sizeof(D3DVERTEX);
+    exec_data.dwInstructionLength = 10;
+    hr = IDirect3DExecuteBuffer_SetExecuteData(execute_buffer, &exec_data);
+    ok(SUCCEEDED(hr), "Failed to set execute data, hr %#x.\n", hr);
+
+    /* dwSize is checked against the expected struct size. */
+    exec_data.dwSize = sizeof(exec_data) - 1;
+    hr = IDirect3DExecuteBuffer_SetExecuteData(execute_buffer, &exec_data);
+    ok(hr == DDERR_INVALIDPARAMS, "Got unexpected hr %#x.\n", hr);
+    exec_data.dwSize = sizeof(exec_data) + 1;
+    hr = IDirect3DExecuteBuffer_SetExecuteData(execute_buffer, &exec_data);
+    ok(hr == DDERR_INVALIDPARAMS, "Got unexpected hr %#x.\n", hr);
+
+    /* The rest of the data is not checked for plausibility. */
+    exec_data.dwSize = sizeof(exec_data);
+    exec_data.dwVertexCount = 0;
+    hr = IDirect3DExecuteBuffer_SetExecuteData(execute_buffer, &exec_data);
+    ok(SUCCEEDED(hr), "Failed to set execute data, hr %#x.\n", hr);
+    exec_data.dwVertexCount = exec_desc.dwBufferSize / sizeof(D3DVERTEX) - 1;
+    hr = IDirect3DExecuteBuffer_SetExecuteData(execute_buffer, &exec_data);
+    ok(SUCCEEDED(hr), "Failed to set execute data, hr %#x.\n", hr);
+    exec_data.dwVertexCount = exec_desc.dwBufferSize / sizeof(D3DVERTEX);
+    hr = IDirect3DExecuteBuffer_SetExecuteData(execute_buffer, &exec_data);
+    ok(SUCCEEDED(hr), "Failed to set execute data, hr %#x.\n", hr);
+    exec_data.dwVertexCount = exec_desc.dwBufferSize / sizeof(D3DVERTEX) + 1;
+    hr = IDirect3DExecuteBuffer_SetExecuteData(execute_buffer, &exec_data);
+    ok(SUCCEEDED(hr), "Failed to set execute data, hr %#x.\n", hr);
+    exec_data.dwVertexCount = 999999;
+    hr = IDirect3DExecuteBuffer_SetExecuteData(execute_buffer, &exec_data);
+    ok(SUCCEEDED(hr), "Failed to set execute data, hr %#x.\n", hr);
+    exec_data.dwInstructionOffset = 999999 * sizeof(D3DVERTEX);
+    hr = IDirect3DExecuteBuffer_SetExecuteData(execute_buffer, &exec_data);
+    ok(SUCCEEDED(hr), "Failed to set execute data, hr %#x.\n", hr);
+    exec_data.dwInstructionLength = 10240;
+    hr = IDirect3DExecuteBuffer_SetExecuteData(execute_buffer, &exec_data);
+    ok(SUCCEEDED(hr), "Failed to set execute data, hr %#x.\n", hr);
+
+    /* The input structure is not modified. */
+    ok(exec_data.dwSize == sizeof(exec_data), "Got unexpected struct size %u\n",
+            exec_data.dwSize);
+    ok(exec_data.dwVertexCount == 999999, "Got unexpected vertex count %u\n",
+            exec_data.dwVertexCount);
+    ok(exec_data.dwInstructionOffset == 999999 * sizeof(D3DVERTEX), "Got unexpected instruction offset %u\n",
+            exec_data.dwInstructionOffset);
+    ok(exec_data.dwInstructionLength == 10240, "Got unexpected instruction length %u\n",
+            exec_data.dwInstructionLength);
+
+    /* No validation in GetExecuteData. */
+    memset(&exec_data, 0, sizeof(exec_data));
+    exec_desc.dwSize = sizeof(exec_desc);
+    hr = IDirect3DExecuteBuffer_GetExecuteData(execute_buffer, &exec_data);
+    ok(SUCCEEDED(hr), "Failed to get execute data, hr %#x.\n", hr);
+
+    ok(exec_data.dwSize == sizeof(exec_data), "Got unexpected struct size %u\n",
+            exec_data.dwSize);
+    ok(exec_data.dwVertexCount == 999999, "Got unexpected vertex count %u\n",
+            exec_data.dwVertexCount);
+    ok(exec_data.dwInstructionOffset == 999999 * sizeof(D3DVERTEX), "Got unexpected instruction offset %u\n",
+            exec_data.dwInstructionOffset);
+    ok(exec_data.dwInstructionLength == 10240, "Got unexpected instruction length %u\n",
+            exec_data.dwInstructionLength);
+
+    memset(&exec_data, 0xaa, sizeof(exec_data));
+    exec_desc.dwSize = sizeof(exec_desc) - 1;
+    hr = IDirect3DExecuteBuffer_GetExecuteData(execute_buffer, &exec_data);
+    ok(SUCCEEDED(hr), "Failed to get execute data, hr %#x.\n", hr);
+    ok(exec_data.dwSize == sizeof(exec_data), "Got unexpected struct size %u\n",
+       exec_data.dwSize);
+    ok(exec_data.dwVertexCount == 999999, "Got unexpected vertex count %u\n",
+       exec_data.dwVertexCount);
+    ok(exec_data.dwInstructionOffset == 999999 * sizeof(D3DVERTEX), "Got unexpected instruction offset %u\n",
+       exec_data.dwInstructionOffset);
+    ok(exec_data.dwInstructionLength == 10240, "Got unexpected instruction length %u\n",
+       exec_data.dwInstructionLength);
+
+    exec_desc.dwSize = 0;
+    hr = IDirect3DExecuteBuffer_GetExecuteData(execute_buffer, &exec_data);
+    ok(SUCCEEDED(hr), "Failed to get execute data, hr %#x.\n", hr);
+    exec_desc.dwSize = sizeof(exec_desc) + 1;
+    hr = IDirect3DExecuteBuffer_GetExecuteData(execute_buffer, &exec_data);
+    ok(SUCCEEDED(hr), "Failed to get execute data, hr %#x.\n", hr);
+    exec_desc.dwSize = ~0U;
+    hr = IDirect3DExecuteBuffer_GetExecuteData(execute_buffer, &exec_data);
+    ok(SUCCEEDED(hr), "Failed to get execute data, hr %#x.\n", hr);
+
+    IDirect3DExecuteBuffer_Release(execute_buffer);
+    IDirect3DDevice_Release(device);
+    IDirectDraw_Release(ddraw);
+    DestroyWindow(window);
+}
+
 START_TEST(ddraw1)
 {
     DDDEVICEIDENTIFIER identifier;
@@ -11348,4 +11474,5 @@ START_TEST(ddraw1)
     test_depth_readback();
     test_clear();
     test_enum_surfaces();
+    test_execute_data();
 }
-- 
2.16.1




More information about the wine-devel mailing list