passing BSTR via IDispatchAPI:invoke will randomly crash IE

[GoogleCodeExporter]

What steps will reproduce the problem?
1. calling a function with string parameter via IDispatchAPI:invoke 
2. it will randomly crash IE (the Maxthon 2.5.12.4586 is more likely to happen)

What is the expected output? What do you see instead?

What version of FireBreath are you using? On what operating system and
browsers?
Latest git

What version of your compiler or IDE are you using?

Please provide any additional information below.
----------------------
I am experiencing this problem for some time. I used to think it is an IE 
bug(because it is less likely to happen in the lastest version of IE), until I 
get it to crash everytime I run a test page. 

So, luckily, I have located the crash location (it is a pretty seriously bug in 
fb code).

in the IDispatchAPI.cpp, you are passing CComVariant array, instead of 
VARIANTARG array. This is not the correct way, because the Invoke method will 
modify the DISPPARAMS.rgvarg array in arbitrary way(perhaps it is using the 
memory for some sorting algorithm). The ms doc also mentions that it is an 
IN/OUT params. I have debugged and verified that the content of 
DISPPARAMS.rgvarg is completely unpredicatable after Invoke returned, so there 
is a chance that CComVariant will release BSTR pointers that point to an 
invalid location (IE crash). 

The fix is that one should always copy CComVariant to VARIANTARG in 
DISPPARAMS.rgvarg, please see the correct code below. 

 boost::scoped_array<CComVariant> comArgs(new CComVariant[args.size()]);
--prepare raw array--> boost::scoped_array<VARIANTARG> comArgs_raw(new 
VARIANTARG[args.size()]);
    DISPPARAMS params;
    params.cArgs = args.size();
    params.cNamedArgs = 0;
--use raw->    params.rgvarg = comArgs_raw.get(); //comArgs.get();

    for (size_t i = 0; i < args.size(); i++) {
        m_browser->getComVariant(&comArgs[args.size() - 1 - i], args[i]);
----copy it without adding ref--->      comArgs_raw[args.size() - 1 - i] = 
comArgs[args.size() - 1 - i];
    }

    CComVariant result;
    CComExcepInfo exceptionInfo;
    HRESULT hr = m_obj->Invoke(dispId, IID_NULL, LOCALE_USER_DEFAULT,
        DISPATCH_METHOD, &params, &result, &exceptionInfo, NULL);
    if (FAILED(hr)) {
        throw FB::script_error("Method invoke failed");
    }

---------->no memory leak, since we use CComVariant, and VARIANTARG is just a 
data copy. 

Original issue reported on code.google.com by xizhi...@gmail.com on 19 Feb 2011 at 3:28

[GoogleCodeExporter]

I have never experienced this, but it's a reasonable explanation and the fix 
seems to work, so I have pulled it into the 1.4 and master branches and it will 
be in 1.4 RC2. (it's there now if you pull from git)

Original comment by richarda...@gmail.com on 19 Feb 2011 at 8:10

• Changed state: Fixed

[GoogleCodeExporter]

ok. thanks

Original comment by xizhi...@gmail.com on 20 Feb 2011 at 2:31