MicrosoftEdge / WebView2Feedback

Feedback and discussions about Microsoft Edge WebView2
https://aka.ms/webview2
440 stars 51 forks source link

Access violation when JS is accessing method or property that returns interface with NULL #147

Closed pontusn closed 4 years ago

pontusn commented 4 years ago

Implement a property that returns an interface:

interface ITestCrash : IDispatch 
{
 [propget]
 HRESULT ActiveDummy([out, retval] IDummy* *pVal)
}

Set *pVal=NULL

Results in access violation in WebView2-host.

pontusn commented 4 years ago

Also for this we have a workaround in IDispatch:Invoke for our internal APIs but for external components there is no easy fix:

    STDMETHOD(Invoke)(
        _In_ DISPID dispidMember,
        _In_ REFIID riid,
        _In_ LCID lcid,
        _In_ WORD wFlags,
        _In_ DISPPARAMS* pdispparams,
        _Out_opt_ VARIANT* pvarResult,
        _Out_opt_ EXCEPINFO* pexcepinfo,
        _Out_opt_ UINT* puArgErr)
    {
        // Resolve ambuities for indexed properties and methods without arguments
        if ((wFlags & (DISPATCH_METHOD | DISPATCH_PROPERTYGET)) != 0L)
        {
            wFlags |= DISPATCH_METHOD | DISPATCH_PROPERTYGET;
        }

        HRESULT hRes = __super::Invoke(dispidMember, riid, lcid,
            wFlags, pdispparams, pvarResult, pexcepinfo, puArgErr);
        if (SUCCEEDED(hRes) && pvarResult != nullptr)
        {
            // Avoid returning NULL-pointers for interfaces since some API-clients crash
            if ((pvarResult->vt == VT_DISPATCH && pvarResult->pdispVal == nullptr) ||
                (pvarResult->vt == VT_UNKNOWN && pvarResult->punkVal == nullptr))
            {
                pvarResult->vt = VT_NULL;
            }
        }
        return hRes;
    }
david-risney commented 4 years ago

Is that return S_OK and set the out pointer to nullptr?

We'll look into it. Thanks for reporting this issue!

champnic commented 4 years ago

This should be fixed in Edge 84.0.492.0+. Thanks!