WineD3D: heap corruption in d3dfmt_convert_surface

[Markus Amsler]

I have (finally!) found the heap corruption in bf1942:
Small surfaces have a pitch of 4 and a width <4. This results in pitch*4
> outpitch in IWineD3DSurfaceImpl_LoadTexture:1844. Afterwards the heap

gets corrupted in d3dfmt_convert_surface:1654. The attached patch fixes 
the problem.
I'm quite sure this fix is ugly, Suggestions?

Markus

[Stefan Dösinger]

Am Montag 06 November 2006 15:44 schrieb Markus Amsler:
> I have (finally!) found the heap corruption in bf1942:
> Small surfaces have a pitch of 4 and a width <4. This results in pitch*4
> > outpitch in IWineD3DSurfaceImpl_LoadTexture:1844. Afterwards the heap
>
> gets corrupted in d3dfmt_convert_surface:1654. The attached patch fixes
> the problem.
> I'm quite sure this fix is ugly, Suggestions?

Yeah, such a hack is just ugly, and convert_surface depends on the correct in 
and outpitch.

First of all, I wonder why bf1942 ends up using converted surfaces. As a d3d8 
game it can't do color keying, and I have never seen it using palettized 
textures.

I think the bug is rather in d3dfmt_convert_surface. In case of a 1x1 
surface(pitch=4) it converts pitch bytes, giving 16 output bytes instead of 
4. I think the correct solution would be to additionally pass in the with of 
the surface and use it for controlling the conversion loops, while using the 
pitch to find out where to read the source data from. Alternatively a check 
based on the outpitch could cause the loops to switch to the next line.

outpitch = (outpitch + 3) & ~3;
The 3 in this line should be replaced by (SURFACE_ALIGNMENT - 1) too.