[PATCH 6/6] d3dx9: Secure against unsafe iface to COM object transitions
Paul Gofman
gofmanp at gmail.com
Fri Mar 22 10:11:05 CDT 2019
On 3/22/19 17:25, Paul Gofman wrote:
> On 3/22/19 16:37, Chip Davis wrote:
>> March 22, 2019 2:49 AM, "Michael Stefaniuc" <mstefani at winehq.org> wrote:
>>
>>> @@ -6162,7 +6164,7 @@ static HRESULT d3dx9_effect_init(struct
>>> d3dx_effect *effect, struct IDirect3DDev
>>> if (pool)
>>> {
>>> pool->lpVtbl->AddRef(pool);
>>> - effect->pool = impl_from_ID3DXEffectPool(pool);
>>> + effect->pool = unsafe_impl_from_ID3DXEffectPool(pool);
>>> }
>> Now you are leaking 'pool' in case it's not one of ours.
>>
>> Chip
>>
>>
> This can't really work if the pool is not one of ours. Pool does not
> have a public interface to work with from the effect's side. We don't
> have any tests showing what native d3dx9 will do in such a case: crash
> (I would put my bet on this before testing), return an error or act if
> no pool is provided, or even use the pool pointer just like a hash
> value and work fine with any object claimed to be a pool??. IMO unless
> we want to add some tests for this case having NULL from
> unsafe_impl_from_ID3DXEffectPool() deserves a FIXME at the first
> place. Is silently acting as if there is no pool better than crashing
> or leaking something pretending to be a pool object, unless we are
> sure this is a right thing to do?
>
FWIW I just made a quick test setting pool->lpVtbl to NULL in pool
before using this pool in D3DXCreateEffect, and native d3dx9_36.dll
crashes on read access to address 0x4, just the same way as builtin
currently does.
More information about the wine-devel
mailing list