IDA decompiler's Control Flow Guard check ignore function has a bug

IDA General
1

Compile the following code with vc x86 Release /Od /guard:cf:


#define WIN32_LEAN_AND_MEAN
#include <windows.h>
#include <unknwn.h>

class CUnknown :public IUnknown
{
public:
	CUnknown()
	{
	}
public:
	virtual HRESULT STDMETHODCALLTYPE QueryInterface(
		/* [in] */ REFIID riid,
		/* [iid_is][out] */ _COM_Outptr_ void __RPC_FAR* __RPC_FAR* ppvObject)
	{
		return S_OK;
	}
	ULONG STDMETHODCALLTYPE AddRef()
	{
		return 1;
	}
	ULONG STDMETHODCALLTYPE Release()
	{
		return 0;
	}
};
int main()
{
	IUnknown* p = new CUnknown();
	p->Release();
}

Use IDA to load the compiled exe and automatically load pdb. You will see the main function as follows:

int __cdecl main()
{
  CUnknown *v1; // [esp+10h] [ebp-14h]
  CUnknown *v2; // [esp+14h] [ebp-10h]

  v2 = (CUnknown *)operator new(4u);
  if ( v2 )
    v1 = CUnknown::CUnknown(v2);
  else
    v1 = 0;
  ((void (__thiscall *)(unsigned int (__stdcall *)(IUnknown *), CUnknown *))v1->Release)(v1->Release, v1);
  return 0;
}

As you can see above, the Release call passes an extra parameter - v1->Release

Reference documents:

2

Thank you for the report and the sample! Indeed, it seems the call is wrongly treated as __thiscall, probably because ecx is used as intermediate variable for the pointer value.

3

Yes, you are right. Maybe this problem has nothing to do with the Control Flow Guard check ignore function. It is just a problem with IDA’s analysis logic. But how can I correct this decompilation error? There are a lot of such recognition errors in the samples I want to analyze, so I need to automate batch processing.

4

I think “Set call type” might work, but the simplest solution would probably be to just nop out calls to ___guard_check_icall_fptr and maybe the preceding manipulations with ecx.

5

Thanks for your suggestion, but I found

nop out calls to ___guard_check_icall_fptr and maybe the preceding manipulations with ecx

is not a reliable method. Here is an example:
Change the main function to the following code,

int main()
{
	IUnknown* p = new CUnknown();
	p->AddRef();
	p->QueryInterface(GUID_NULL, NULL);
	p->Release();
	return 0;
}

and then compile it after selecting LLVM (clang-cl) as the platform toolset. You will see that its disassembly is:

.text:00401000 _main           proc near               ; CODE XREF: __scrt_common_main_seh+F8↓p
.text:00401000
.text:00401000 var_3C          = dword ptr -3Ch
.text:00401000 size            = dword ptr -34h
.text:00401000 var_30          = dword ptr -30h
.text:00401000 var_2C          = dword ptr -2Ch
.text:00401000 var_28          = dword ptr -28h
.text:00401000 var_24          = dword ptr -24h
.text:00401000 var_20          = dword ptr -20h
.text:00401000 var_1C          = dword ptr -1Ch
.text:00401000 var_18          = dword ptr -18h
.text:00401000 var_14          = dword ptr -14h
.text:00401000 var_10          = dword ptr -10h
.text:00401000 var_C           = dword ptr -0Ch
.text:00401000 var_8           = dword ptr -8
.text:00401000
.text:00401000                 push    ebp
.text:00401001                 mov     ebp, esp
.text:00401003                 push    esi
.text:00401004                 sub     esp, 30h
.text:00401007                 mov     [ebp+var_8], 0
.text:0040100E                 mov     [esp+34h+size], 4 ; size
.text:00401015                 call    ??2@YAPAXI@Z    ; operator new(uint)
.text:0040101A                 mov     ecx, eax
.text:0040101C                 mov     [ebp+var_28], ecx
.text:0040101F                 call    ??0CUnknown@@QAE@XZ ; CUnknown::CUnknown(void)
.text:00401024                 mov     eax, [ebp+var_28]
.text:00401027                 mov     [ebp+var_C], eax
.text:0040102A                 mov     eax, [ebp+var_C]
.text:0040102D                 mov     [ebp+var_24], eax
.text:00401030                 mov     eax, [eax]
.text:00401032                 mov     ecx, [eax+4]    ; Target
.text:00401035                 mov     [ebp+var_20], ecx
.text:00401038                 call    ds:___guard_check_icall_fptr ; _guard_check_icall_nop(x) ...
.text:0040103E                 mov     ecx, [ebp+var_24]
.text:00401041                 mov     eax, [ebp+var_20]
.text:00401044                 mov     [esp+34h+size], ecx
.text:00401047                 call    eax
.text:00401049                 sub     esp, 4
.text:0040104C                 mov     eax, [ebp+var_C]
.text:0040104F                 mov     [ebp+var_1C], eax
.text:00401052                 mov     eax, [eax]
.text:00401054                 mov     ecx, [eax]      ; Target
.text:00401056                 mov     [ebp+var_18], ecx
.text:00401059                 call    ds:___guard_check_icall_fptr ; _guard_check_icall_nop(x) ...
.text:0040105F                 mov     edx, [ebp+var_1C]
.text:00401062                 mov     eax, [ebp+var_18]
.text:00401065                 lea     ecx, _GUID_NULL
.text:0040106B                 xor     esi, esi
.text:0040106D                 mov     [esp+34h+size], edx
.text:00401070                 mov     [esp+34h+var_30], ecx
.text:00401074                 mov     [esp+34h+var_2C], 0
.text:0040107C                 call    eax
.text:0040107E                 sub     esp, 0Ch
.text:00401081                 mov     eax, [ebp+var_C]
.text:00401084                 mov     [ebp+var_14], eax
.text:00401087                 mov     eax, [eax]
.text:00401089                 mov     ecx, [eax+8]    ; Target
.text:0040108C                 mov     [ebp+var_10], ecx
.text:0040108F                 call    ds:___guard_check_icall_fptr ; _guard_check_icall_nop(x) ...
.text:00401095                 mov     ecx, [ebp+var_14]
.text:00401098                 mov     eax, [ebp+var_10]
.text:0040109B                 mov     [esp+3Ch+var_3C], ecx
.text:0040109E                 call    eax
.text:004010A0                 sub     esp, 4
.text:004010A3                 xor     eax, eax
.text:004010A5                 add     esp, 30h
.text:004010A8                 pop     esi
.text:004010A9                 pop     ebp
.text:004010AA                 retn
.text:004010AA _main           endp

As you can see,

.text:00401032                 mov     ecx, [eax+4]    ; Target
.text:00401035                 mov     [ebp+var_20], ecx
.text:00401038                 call    ds:___guard_check_icall_fptr ; _guard_check_icall_nop(x) ...

There are other instructions between mov ecx and call ds:__guard_check_icall_fptr, and the middle instruction uses the value of ecx to be used by the following instructions.

Here’s another problem: Change the platform toolset to Visual Studio 2010 (v100), you will see the following pseudo code:

int __cdecl main(int argc, const char **argv, const char **envp)
{
  CUnknown *v4; // [esp+0h] [ebp-10h]
  CUnknown *v5; // [esp+4h] [ebp-Ch]
  IUnknown *p; // [esp+Ch] [ebp-4h]

  v5 = (CUnknown *)operator new(4u);
  if ( v5 )
    v4 = CUnknown::CUnknown(v5);
  else
    v4 = 0;
  p = v4;
  ((void (__thiscall *)(CUnknown *, CUnknown *))v4->AddRef)(v4, v4);
  p->QueryInterface(p, &GUID_NULL, 0);
  p->Release(p);
  return 0;
}

Visual Studio 2010 does not support the Control Flow Guard feature, so there is no ___guard_check_icall_fptr call, but IDA still misidentifies the first AddRef call. This seems to be a problem caused by the changes in IDA7.1, because I can get the correct output when decompiling with IDA7.0, as follows:

int __cdecl main(int argc, const char **argv, const char **envp)
{
  IUnknown *v3; // eax
  IUnknown *p; // ST18_4
  IUnknown *v6; // [esp+0h] [ebp-10h]
  CUnknown *v7; // [esp+4h] [ebp-Ch]

  v7 = (CUnknown *)operator new(4u);
  if ( v7 )
  {
    CUnknown::CUnknown(v7);
    v6 = v3;
  }
  else
  {
    v6 = 0;
  }
  p = v6;
  v6->vfptr->AddRef(v6);
  p->vfptr->QueryInterface(p, &GUID_NULL, 0);
  p->vfptr->Release(p);
  return 0;
}

Therefore, I think there is a problem with the latest IDA’s judgment logic for the __thiscall calling convention, especially for the scenario of virtual table function calling. In this scenario, should we strictly abide by the calling convention in the virtual table function declaration, If this information is provided through debugging symbols such as PDB/DWARF? I hope that the next version of IDA can improve the judgment logic for __thiscall.
However, here, Force call type should be a mitigation measure.

6

Interestingly, the Clang compiled file seems to produce a correct output for me, although I get a slightly different assembly.

int __cdecl main()
{
  CUnknown *v1; // [esp+4h] [ebp-10h]

  v1 = (CUnknown *)operator new(4u);
  CUnknown::CUnknown(v1);
  v1->Release(v1);
  return 0;
}
.text:00414B98                 mov     eax, [eax]
.text:00414B9A                 mov     ecx, [eax+8]    ; Target
.text:00414B9D                 mov     [ebp+var_8], ecx
.text:00414BA0                 call    ds:___guard_check_icall_fptr ; _guard_check_icall_nop(x) ...
.text:00414BA6                 mov     ecx, [ebp+var_C]
.text:00414BA9                 mov     eax, [ebp+var_8]
.text:00414BAC                 mov     [esp+14h+size], ecx
.text:00414BAF                 call    eax
.text:00414BB1                 sub     esp, 4

In general, it’s best to always provide the actual files/idb you’re working with, as even seemingly minor differences can matter.

7

Because you didn’t change the code of the main function first as I said, and I compiled it with clang 20.1.5 Release /Od /guard:cf, you can try it again. In addition, you can also pay attention to the situation of using vs2010 to compile in my last reply. 7.0 is normal, and 7.1 has an additional layer of forced conversion.
Thanks for reply.

8

We would really appreciate if you uploaded compiled binaries, as hunting down the exact compiler versions and switches is quite time consuming. So far, nopping out the call to ___guard_check_icall_fptr seems to be enough in most cases (with occasional “Force call type”).

9

If you need binary, you’ll have to wait a few days because I’m currently on vacation and don’t have my work computer with me. Thank you.

10

Hello, I have uploaded the relevant demo exe, pdb, idb.
source code：

int main()
{
	IUnknown* p = new CUnknown();
	p->AddRef();
	p->QueryInterface(GUID_NULL, NULL);
	p->Release();
	return 0;
}

clang:
IdaCfgBugTest-clang.7z

int __cdecl main()
{
  int v1; // [esp+Ch] [ebp-38h]
  unsigned int size; // [esp+10h] [ebp-34h]
  IUnknown *v3; // [esp+1Ch] [ebp-28h]

  v3 = (IUnknown *)operator new(4u);
  CUnknown::CUnknown((CUnknown *)v3);
  v3->AddRef(v3);
  ((void (__stdcall *)(IUnknown *))v3->QueryInterface)(v3);
  ((void (__stdcall *)(IUnknown *, int, unsigned int))v3->Release)(v3, v1, size);
  return 0;
}

vs2010:
IdaCfgBugTest-vs2010.7z

int __cdecl main(int argc, const char **argv, const char **envp)
{
  CUnknown *v4; // [esp+0h] [ebp-10h]
  CUnknown *v5; // [esp+4h] [ebp-Ch]
  IUnknown *p; // [esp+Ch] [ebp-4h]

  v5 = (CUnknown *)operator new(4u);
  if ( v5 )
    v4 = CUnknown::CUnknown(v5);
  else
    v4 = 0;
  p = v4;
  ((void (__thiscall *)(CUnknown *, CUnknown *))v4->AddRef)(v4, v4);
  p->QueryInterface(p, &GUID_NULL, 0);
  p->Release(p);
  return 0;
}
11

Thank you very much for the binaries! It seems “Force call type” helps in both situations. We’ll try to see if it can be improved, but you can read here why it can be difficult to detect proper arguments for indirect calls: Force call type | Hex-Rays Docs

12

I understand that perfectly, but the weird thing is that as mentioned before, the vs2010 example IdaCfgBugTest-vs2010.7z were fine in IDA 7.0, and the cast was present in later versions, so I think it’s a regression bug.

int __cdecl main(int argc, const char **argv, const char **envp)
{
  IUnknown *v3; // eax
  IUnknown *p; // ST18_4
  IUnknown *v6; // [esp+0h] [ebp-10h]
  CUnknown *v7; // [esp+4h] [ebp-Ch]

  v7 = (CUnknown *)operator new(4u);
  if ( v7 )
  {
    CUnknown::CUnknown(v7);
    v6 = v3;
  }
  else
  {
    v6 = 0;
  }
  p = v6;
  v6->vfptr->AddRef(v6);
  p->vfptr->QueryInterface(p, &GUID_NULL, 0);
  p->vfptr->Release(p);
  return 0;
}
13

Thank you. Indeed, it’s possible there was a regression in this specific case, but likely it came with improvements elsewhere.