Incorrect COM interface types generated from type libraries

[alexrp]

Version and Platform (required):

• Binary Ninja Version: 3.1.3478-dev
• OS: Windows
• OS Version: 10.0.25126.1000
• CPU Architecture: x64

Bug Description: Binary Ninja's handling of COM interfaces in type libraries is wrong in several ways:

• Function pointers in vtables don't have a this pointer.
• Inheritance is not handled, leading to vtables only having methods defined directly on themselves.
  • In some cases, this can lead to empty vtables.
  • In other cases, it can lead to intermediate interface vtables not even being generated.
• COM interface pointers in signatures are missing a level of indirection. E.g. a pointer to IUnknown will show up as just IUnknown rather than IUnknown*.

Steps To Reproduce: Analyze basically any binary that uses Direct3D and look at the (nano-)COM structs generated from the relevant type libraries.

Example:

typedef struct _IUnknown* IUnknown;

struct _IUnknown
{
    HRESULT (* QueryInterface)(struct GUID* riid, void** ppvObject);
    uint32_t (* AddRef)();
    uint32_t (* Release)();
};

typedef struct _ID3D11ComputeShader* ID3D11ComputeShader;

struct _ID3D11ComputeShader
{
};

HRESULT (* const d3d11:D3D11CreateDevice)(
    IDXGIAdapter pAdapter,
    enum D3D_DRIVER_TYPE DriverType,
    HINSTANCE Software,
    enum D3D11_CREATE_DEVICE_FLAG Flags,
    enum D3D_FEATURE_LEVEL* pFeatureLevels,
    uint32_t FeatureLevels,
    uint32_t SDKVersion,
    ID3D11Device* ppDevice,
    enum D3D_FEATURE_LEVEL* pFeatureLevel,
    ID3D11DeviceContext* ppImmediateContext);

Expected Behavior: I would expect something more like this:

typedef struct _IUnknown* IUnknown;

struct _IUnknown
{
    HRESULT (* QueryInterface)(IUnknown* `this`, struct GUID* riid, void** ppvObject);
    uint32_t (* AddRef)(IUnknown* `this`);
    uint32_t (* Release)(IUnknown* `this`);
};

typedef struct _ID3D11DeviceChild* ID3D11DeviceChild;

struct _ID3D11DeviceChild
{
    struct _IUnknown __base;
    void (* GetDevice)(ID3D11DeviceChild* `this`, ID3D11Device** ppDevice);
    HRESULT (* GetPrivateData)(ID3D11DeviceChild* `this`, struct GUID* guid, uint32_t* pDataSize, void* pData);
    HRESULT (* SetPrivateData)(ID3D11DeviceChild* `this`, struct GUID* guid, uint32_t DataSize, void const* pData);
    HRESULT (* SetPrivateDataInterface)(ID3D11DeviceChild* `this`, struct GUID* guid, IUnknown* pData);
};

typedef struct _ID3D11ComputeShader* ID3D11ComputeShader;

struct _ID3D11ComputeShader
{
    struct _ID3D11DeviceChild __base;
};

HRESULT (* const d3d11:D3D11CreateDevice)(
    IDXGIAdapter* pAdapter,
    enum D3D_DRIVER_TYPE DriverType,
    HINSTANCE Software,
    enum D3D11_CREATE_DEVICE_FLAG Flags,
    enum D3D_FEATURE_LEVEL* pFeatureLevels,
    uint32_t FeatureLevels,
    uint32_t SDKVersion,
    ID3D11Device** ppDevice,
    enum D3D_FEATURE_LEVEL* pFeatureLevel,
    ID3D11DeviceContext** ppImmediateContext);

[plafosse]

Do you have a reference to this?

[alexrp]

Do you have a reference to this?

Sorry, what do you mean by reference here?

[psifertex]

I assume he means a link to the official definition.

[alexrp]

I only know of the MSDN docs:

• https://docs.microsoft.com/en-us/windows/win32/api/unknwn/nn-unknwn-iunknown
• https://docs.microsoft.com/en-us/windows/win32/api/d3d11/nn-d3d11-id3d11devicechild
• etc

[plafosse]

@alexrp I believe I have fixed this in the current set of type libraries that are on dev. I don't know the exact build number this was fixed in but 3.6.4745 should be good.