[PATCH 0/1] MR5272: ddraw: Reserve extra space in the reference device description buffer.

[Andrew Nguyen (＠arethusa)]

During Direct3D device enumeration, Tomb Raider 3 modifies the buffer holding the Direct3D reference device description string. However, due to insufficient space in the buffer, the modification overruns the buffer and corrupts the IDirectDraw vtable, which leads to a crash on game startup.

Reserving extra space in the description string buffer avoids the vtable corruption and allows the game to start.

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

[PATCH 1/1] ddraw: Reserve extra space in the reference device description buffer.

[Andrew Nguyen]

From: Andrew Nguyen

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56367
---
 dlls/ddraw/ddraw.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/dlls/ddraw/ddraw.c b/dlls/ddraw/ddraw.c
index 5887854556b..efbe01f3b8b 100644
--- a/dlls/ddraw/ddraw.c
+++ b/dlls/ddraw/ddraw.c
@@ -3868,7 +3868,10 @@ static HRESULT WINAPI d3d3_EnumDevices(IDirect3D3 *iface, LPD3DENUMDEVICESCALLBA
      * never have POW2 unset in d3d7 on windows. */
     if (ddraw->d3dversion != 1)
     {
-        static CHAR reference_description[] = "RGB Direct3D emulation";
+        /* Tomb Raider 3 overwrites the reference device description buffer
+         * with its own custom string. Reserve some extra space in the array
+         * to avoid a buffer overrun. */
+        static CHAR reference_description[64] = "RGB Direct3D emulation";

TRACE("Enumerating WineD3D D3DDevice interface.\n");
         hal_desc = device_desc1;

[Zebediah Figura (＠zfigura)]

Thanks for the patch!

Do we know how much space the game requires exactly? I don't want to risk being still too low here.

[Andrew Nguyen (＠arethusa)]

On Mon Mar 11 20:50:28 2024 +0000, Zebediah Figura wrote:
> Thanks for the patch!
> Do we know how much space the game requires exactly? I don't want to
> risk being still too low here.

From looking at the output of the strings command on the game executable, I think it would overwrite the description buffer either with "Core Design MMX Hardware Card Emulation" or "Core Design RGB Hardware Card Emulation" which both would occupy 40 bytes (including the null terminator).

I chose a size of 64 bytes for the reference_description buffer as a conservative estimate because native can report for the Direct3D HAL device the description string "Microsoft Direct3D Hardware acceleration through Direct3D HAL" which occupies 62 bytes (including the null terminator). I can make it larger if necessary, but 64 bytes should be more than enough for the game.

Speaking of which, should the wined3d_description buffer be expanded in the same way? I don't know of any application that needs to overwrite it with more than 40 bytes, but native can report a larger description string as I mentioned.

[Zebediah Figura (＠zfigura)]

On Mon Mar 11 20:50:28 2024 +0000, Andrew Nguyen wrote:
> From looking at the output of the strings command on the game
> executable, I think it would overwrite the description buffer either
> with "Core Design MMX Hardware Card Emulation" or "Core Design RGB
> Hardware Card Emulation" which both would occupy 40 bytes (including the
> null terminator).
> I chose a size of 64 bytes for the reference_description buffer as a
> conservative estimate because native can report for the Direct3D HAL
> device the description string "Microsoft Direct3D Hardware acceleration
> through Direct3D HAL" which occupies 62 bytes (including the null
> terminator). I can make it larger if necessary, but 64 bytes should be
> more than enough for the game.
> Speaking of which, should the wined3d_description buffer be expanded in
> the same way? I don't know of any application that needs to overwrite it
> with more than 40 bytes, but native can report a larger description
> string as I mentioned.

64 sounds good to me then, I just want to make sure we're not increasing to some arbitrary number that itself might not be enough.
> Speaking of which, should the wined3d_description buffer be expanded in the same way? I don't know of any application that needs to overwrite it with more than 40 bytes, but native can report a larger description string as I mentioned.

I wouldn't worry about it if there's no evidence that applications will overwrite. "What if" scenarios are uncountable, after all ;-)

[PATCH 0/1] MR5272: ddraw: Reserve extra space in the reference device description buffer. - approved

[Zebediah Figura (＠zfigura)]

This merge request was approved by Zebediah Figura.

[PATCH 0/1] MR5272: ddraw: Reserve extra space in the reference device description buffer. - approved

[Jan Sikorski (＠jsikorski)]

This merge request was approved by Jan Sikorski.