microsoft / binskim

A binary static analysis tool that provides security and correctness results for Windows Portable Executable and *nix ELF binary formats
Other
779 stars 156 forks source link

False positives with BA2008 IMAGE_DLLCHARACTERISTICS_CONTROLFLOWGUARD check #804

Closed connorcashmsft closed 1 year ago

connorcashmsft commented 1 year ago

Hi, I have a binary that is being flagged as missing CFG despite it being compiled with /guard:cf. In binskim, the check is returning false here:

            if (((uint)peHeader.DllCharacteristics & IMAGE_DLLCHARACTERISTICS_CONTROLFLOWGUARD) == 0)
            {
                return false;
            }

I wasn't sure if this bit was not set by the linker or compiler due to missing CFG in a static library that is linked into the final binary. So, I opened the binary in a disassembler and wrote a script to look for all indirect calls that aren't preceded by a CFG check. Out of approximately 14k calls, only 5 were missing the check. So I'm assuming all of the dependency static libraries have CFG too.

Do you know what else could cause this DllCharacteristics bit to not get set? Is it intended that a binary with anything less than 100% CFG-checked indirect calls will fail?

connorcashmsft commented 1 year ago

I manually looked into the 5 calls my script reported as unchecked, and it turns out those are actually checked by CFG too. The check just occurs a little further up the control flow graph and my script is not smart enough to detect those. So this binary definitely should pass.

connorcashmsft commented 1 year ago

I suspect this is actually. due to the binary missing /DYNAMICBASE. I haven't been able to get a full list of compile flags from it, but I'll investigate more when I have time.

shaopeng-gh commented 1 year ago

Thanks for sending the repo file, I have cross-checked with another tool dumpbin /headers /loadconfig the DLL characteristics read by BinSkim look correct:

image

image

A file that passing the rule will be like:

image

Looks like it is not a BinSkim false positive. I have no insight how the binary is built though.

Thanks for reporting,

shaopeng-gh commented 1 year ago

@connorcashmsft let me know if we can close the issue, thanks.

connorcashmsft commented 1 year ago

Ah, sorry! Yes, I think we can close this. Thanks for looking into this!