‎

Ghidra: the story of _check_sec_cookie

Problem

Earlier this week I came accross an intricate problem while using Ghidra's decompiler on a x86_64 binary compiled by Visual Studio. Before starting, all the code snippets are copy-pasted from Ghidra's decompiler except for the assembly one which is from the Listing view.

Back to describing the problem now. In the following code snippet, you can see that FUNC function returns an int type variable.

{
  . . .
  int iVar4;
  . . .
  _DAT_1800e2cf0 = (uint)(iVar4 == 0x0);
  if ((iVar4 == 0x0) && (iVar4 = FUNC(), iVar4 != 0x0)) {
    DAT_1800cff98 = 0x0;
  }
  . . .
}

void FUNC(void)
{
  int tmp1;
  int tmp2;
  ulonglong stack_canary;
  wchar_t *cmdParams;
  ulonglong rsp;
  wchar_t executablePath [0x104];

  stack_canary = rand_canary ^ (ulonglong)&rsp;
  GetModuleFileNameW(NULL,executablePath,0x104);
  tmp1 = FUN_1800368e8();
  if ((tmp1 != 0x0) ||
     (tmp2 = FUN_180035f74(L"*\\procmon.exe",executablePath), tmp2 == 0x0)) {
    wcscpy(executablePath,localAllocation);
    wcscat(executablePath,L"\\system32\\svchost.exe -k RPCSS");
    cmdParams = GetCommandLineW();
    FUN_180052134(cmdParams,executablePath);
  }
  _check_sec_cookie(stack_canary ^ (ulonglong)&rsp);
  return;
}

Well, the initial auto-analysis decided that FUNC returns a void type. If we try to fix that by editing the function signature and replacing void with int, we get the following:

int FUNC(void)
{
  int tmp1;
  int tmp2;
  ulonglong stack_canary;
  wchar_t *cmdParams;
  ulonglong rsp;
  wchar_t executablePath [0x104];

  stack_canary = rand_canary ^ (ulonglong)&rsp;
  GetModuleFileNameW(NULL,executablePath,0x104);
  tmp1 = FUN_1800368e8();
  if ((tmp1 != 0x0) ||
     (tmp2 = FUN_180035f74(L"*\\procmon.exe",executablePath), tmp2 == 0x0)) {
    wcscpy(executablePath,localAllocation);
    wcscat(executablePath,L"\\system32\\svchost.exe -k RPCSS");
    cmdParams = GetCommandLineW();
    FUN_180052134(cmdParams,executablePath);
  }
  tmp1 = _check_sec_cookie(stack_canary ^ (ulonglong)&rsp);
  return tmp1;
}

So it seems like the int variable that the function FUNC returns is the return value of _check_sec_cookie. In case you know the purpose of _chec_sec_cookie, you know that it can't be right.

If we look at the _check_sec_cookie function declaration, we see that it returns a void type.

void _check_sec_cookie (longlong param_1);

Something seems really odd here.

The _check_sec_cookie function is a compiler generated function that is part of the stack canary mechanism that the compiler implements.

Before moving on, the solution assumes that you know how a stack canary works. If you don't know, take a look at one of the articles referenced.[1][2][3][4][5]

Solution

The lines that are related to the stack canary mechanism are the two lines that contain the stack_canary local variable. The rand_canary variable is a global variable which stores the randomly generated 8-byte value at the start of the process.

The function _check_sec_cookie checks if the stack_canary was corrupted. Since it is part of the epilogue and it is an inserted function call by the compiler it shouldn't clobber the return value of the caller, as it seems to be doing in the last snapshot of the function FUNC earlier.

On a closer look at the _check_sec_cookie function we witness that there are two cases. In the first case (stack_canary is not corrupted) the conditional at the first "if" condition is true and the return statement returns us to the caller. In the second case (stack_canary is corrupted) __raise_securityfailure is executed and calls TerminateProcess[6] which terminates the running process. Even if we mark the __raise_securityfailure as non-returning function, it doesn't solve our problem.

void _check_sec_cookie(longlong param_1)
{
  code *pcVar1;
  BOOL BVar2;
  undefined *puVar3;
  undefined auStack56 [0x8];
  undefined auStack48 [0x30];

  if ((param_1 == rand_canary) && ((short)((ulonglong)param_1 >> 0x30) == 0x0)) {
    return;
  }
  puVar3 = auStack56;
  BVar2 = IsProcessorFeaturePresent(0x17);
  if (BVar2 != 0x0) {
    pcVar1 = (code *)swi(0x29);
    (*pcVar1)(0x2);
    puVar3 = auStack48;
  }
  *(undefined8 *)(puVar3 + -0x8) = 0x180051276;
  capture_previous_context((PCONTEXT)&DAT_1800e1260,puVar3[-0x8]);
  _DAT_1800e11d0 = *(undefined8 *)(puVar3 + 0x38);
  _DAT_1800e12f8 = puVar3 + 0x40;
  _DAT_1800e12e0 = *(undefined8 *)(puVar3 + 0x40);
  _DAT_1800e11c0 = 0xc0000409;
  _DAT_1800e11c4 = 0x1;
  _DAT_1800e11d8 = 0x1;
  DAT_1800e11e0 = 0x2;
  *(longlong *)(puVar3 + 0x20) = rand_canary;
  *(undefined8 *)(puVar3 + 0x28) = DAT_1800cf018;
  *(undefined8 *)(puVar3 + -0x8) = 0x180051318;
  DAT_1800e1358 = _DAT_1800e11d0;
  __raise_securityfailure((_EXCEPTION_POINTERS *)&PTR_DAT_1800a3488,puVar3[-0x8]);
  return;
}

/* Library Function - Single Match
   Name: __raise_securityfailure
   Library: Visual Studio 2015 Release */

void __raise_securityfailure(_EXCEPTION_POINTERS *param_1)
{
  HANDLE hProcess;

  SetUnhandledExceptionFilter(NULL);
  UnhandledExceptionFilter(param_1);
  hProcess = GetCurrentProcess();
		    /* WARNING: Could not recover jumptable at 0x000180051245. Too many
		       branches */
		    /* WARNING: Treating indirect jump as call */
  TerminateProcess(hProcess,STATUS_STACK_BUFFER_OVERRUN);
  return;
}

We have no interest in the second case because the function _check_sec_cookie never returns. Let's see what happens on the assembly level in the first case.

			    ;undefined __fastcall _check_sec_cookie(longlong param_1)
	  ;param_1       longlong           RCX                      
.text:180050d80 483b0d99e...    CMP         param_1,qword ptr [.data:rand_canary]   
.text:180050d87 f27512          JNZ         LAB_180050d9c                           
.text:180050d8a 48c1c110        ROL         param_1,0x10                            
.text:180050d8e 66f7c1ffff      TEST        param_1,0xffff                          
.text:180050d93 f27502          JNZ         LAB_180050d98                           
.text:180050d96 f2c3            RET                                                  
			    LAB_180050d98:                
.text:180050d98 48c1c910        ROR         param_1,0x10                            
			    LAB_180050d9c:                
.text:180050d9c e9ab040000      JMP         LAB_18005124c

This is the program slice related to the first case. As you can see there are two conditions. What's of interest here is that there is a return path to the caller without clobbering the RAX register. So _check_sec_cookie returns normally returning nothing. That's how the caller is able to return whatever resides at the RAX register upon calling _check_sec_cookie.

But how are we supposed to inform the decompiler that _check_sec_cookie never clobbers the RAX register? After searching online for someone that managed to solve this problem the only thing I found was this issue on github[7]. Marcan proposed a solution but I didn't manage to find any ready made solution so I decided to do that by myself.

This is a calling convention issue so the related file is the .cspec file which is documented at Ghidra/Features/Decompiler/src/main/doc/cspec.xml at the source tree[8] or you can view the compiled docs online[9]. All we need to do is create a new calling convention as a copy of __fastcall (this is the calling convention of _check_sec_cookie) in which RAX is a call-preserved register. The file that should be edited is the x86-64-win.cspec. So I copied the __fastcall <prototype> that resides inside the <default_proto> just after it, gave it a different name __fastcall_chkstk and moved the RAX <register> that was inside <killedbycall> to <unaffected>. In this way, I specify that RAX is a call-preserved register.

Now restart ghidra and change calling convention of _check_sec_cookie from __fastcall to __fastcall_chkstk. Also make sure to change the caller function's return variable type to int. The output is a little messier but correct this time.

int FUNC(void)
{
  int tmp1;
  int tmp2;
  ulonglong stack_canary;
  wchar_t *cmdParams;
  ulonglong uVar1;
  ulonglong rsp;
  wchar_t executablePath [0x104];

  stack_canary = rand_canary ^ (ulonglong)&rsp;
  GetModuleFileNameW(NULL,executablePath,0x104);
  tmp1 = FUN_1800368e8();
  if (tmp1 == 0x0) {
    tmp2 = FUN_180035f74(L"*\\procmon.exe",executablePath);
    if (tmp2 == 0x0) goto LAB_18000ceae;
  }
  else {
LAB_18000ceae:
    wcscpy(executablePath,localAllocation);
    wcscat(executablePath,L"\\system32\\svchost.exe -k RPCSS");
    cmdParams = GetCommandLineW();
    uVar1 = FUN_180052134(cmdParams,executablePath);
    if ((int)uVar1 != 0x0) {
      tmp1 = 0x0;
      goto LAB_18000cef0;
    }
  }
  tmp1 = 0x1;
LAB_18000cef0:
  _check_sec_cookie(stack_canary ^ (ulonglong)&rsp);
  return tmp1;
}

The final .cspec file can be found here[10]. Keep in mind that you can do a similar change for the .cspec file of the x86 architecture.

References

[1]: https://en.wikipedia.org/wiki/Stack_buffer_overflow#Stack_canaries

[2]: https://en.wikipedia.org/wiki/Buffer_overflow_protection#Canaries

[3]: https://access.redhat.com/blogs/766093/posts/3548631

[4]: https://docs.microsoft.com/en-us/cpp/build/reference/gs-buffer-security-check?view=msvc-160

[5]: https://mcuoneclipse.com/2019/09/28/stack-canaries-with-gcc-checking-for-stack-overflow-at-runtime/

[6]: https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-terminateprocess

[7]: https://github.com/NationalSecurityAgency/ghidra/issues/28

[8]: https://github.com/NationalSecurityAgency/ghidra/blob/master/Ghidra/Features/Decompiler/src/main/doc/cspec.xml

[9]: https://spinsel.dev/assets/2020-06-17-ghidra-brainfuck-processor-1/ghidra_docs/compiler_spec/index.html

[10]:https://mageirias.com/articles/ghidra/ghidra_a_story_of_check_sec_cookie/x86-64-win.txt

~anastmag