Static code analysis for detecting passing a wchar_t* to BSTR

Question

Since a BSTR is only a typedef for wchar_t* our code base has several (many?) places where string literals are passed to a method expecting a BSTR this can mess up with marshallers or anyone who tries to use any BSTR specific method (e.g. SysStringLen).

Is there any way to statically detect this type of misuse?

I tried compiling with VC10 /Wall and with static code analysis Microsoft All Rules but the following offending piece of code doesn't get flagged by either of them.

void foo(BSTR str)  
{
    std::cout << SysStringLen(str) << std::endl; 
}

int _tmain()
{
    foo(L"Don't do that");
}

---

Update: After trying to vandalize wtypes.h into detecting these kinds of transgressions I've given up.

I tried two paths, both of which I got to work with my sample program above but once I tried a real project they failed.

1. create a class named BSTR but since a VARIANT has a BSTR as a union member the new class couldn't have any constructors or assignment operators this broke every place were NULL was treated as a BSTR. I tried to replace NULL with a type that has conversion operators but after adding dozens of new operators (comparison, conversion etc.) I started to run into ambiguous calls and gave up.
2. I then tried the way suggested by @CashCow and @Hans (makeing BSTR a typedef to another type of pointer). That didn't work either, after adding toBSTR and fromBSTR methods and littering comutil.h (_bstr_t) and other places with conversions I finally got to the point where the compiler choked at headers produced from IDLs (default values are translated to literal wide strings).

In short I've given up on trying to achieve this on my own, if anyone knows of a code analysis tool that can help I would be very happy to hear about it.

Answer 1

I believe Coverity claims to detect these sorts of vulnerabilities. I remember them mentioning COM stuff specifically when they demo'd to a company I worked for.

Their datasheet certainly seems to imply they check for classes of improper BSTR usage. They have a demo-period; you could try it and see if it flags your sample input.

Answer 2

Are you able to change your methods to take _bstr_t or CComBSTR instead?

If not then as a literal is technically a const wchar_t *, if there is a compiler setting to not allow literal->non-const pointer conversion then you can do that.

Failing that, there is a possibility of modifying the definition of BSTR to be unsigned short *. Then if you build all your source, you will get compiler errors wherever a literal is being passed in and you can fix all this code. Then I would suggest changing it back...

Answer 3

Overload all functions with the BSTR and forward them with the appropriate conversion.

void foo( BSTR str )
{
    std::cout << SysStringLen(str) << std::endl; 
}

void foo( const WCHAR *str )
{
    foo( SysAllocString( str ));
}

int _tmain()
{
    foo( L"don't do this" );
    return 0;
}

Or, to generate compiler errors, change all of the parameter types from BSTR to something else and look for the errors:

typedef UINT bar;

void foo( bar _str )
{
    // make the compiler happy below
    BSTR str = (BSTR)_str;
    std::cout << SysStringLen(str) << std::endl;
}

int _tmain()
{
    foo( L"don't do this" );
    foo( (bar)42 );
    return 0;
}

error C2664: 'foo' : cannot convert parameter 1 from 'const wchar_t [14]' to 'bar'

I assume the C2664 error and the 'const wchar_t[]' type identified by the compiler is what you want the compiler to find for every internal call made to the function using the BSTR?

Answer 4

You may try compiling with Clang, it's static/dynamic analysis may find what you are looking for.