trailofbits / winchecksec

Checksec, but for Windows: static detection of security mitigations in executables
https://trailofbits.github.io/winchecksec/
Apache License 2.0
564 stars 77 forks source link

Incorrect reports on binaries built with Visual Studio 2022 #1049

Open avivanoff opened 1 year ago

avivanoff commented 1 year ago

The tool produces incorrect results on the binaries built with Visual Studio 2022.

  1. Build Project1.zip in Release|Win32 configuration.
  2. Run winchecksec on the resultant binary.

At a minimum SafeSEH incorrectly reported as NotPresent when dumpbin /loadconfig shows 4 Safe Exception Handler Count.

woodruffw commented 1 year ago

Thanks for the report. Could you dump your entire dumpbin /loadconfig results?

woodruffw commented 1 year ago

(Also, it looks like you might have forgotten to attach the project.)

avivanoff commented 1 year ago
Microsoft (R) COFF/PE Dumper Version 14.34.31937.0
Copyright (C) Microsoft Corporation.  All rights reserved.

Dump of file .\Project1.exe

File Type: EXECUTABLE IMAGE

  Section contains the following load config:

            000000C0 size
                   0 time date stamp
                0.00 Version
                   0 GlobalFlags Clear
                   0 GlobalFlags Set
                   0 Critical Section Default Timeout
                   0 Decommit Free Block Threshold
                   0 Decommit Total Free Threshold
            00000000 Lock Prefix Table
                   0 Maximum Allocation Size
                   0 Virtual Memory Threshold
                   0 Process Affinity Mask
                   0 Process Heap Flags
                   0 CSD Version
                0000 Dependent Load Flag
            00000000 Edit List
            00403000 Security Cookie
            00402280 Safe Exception Handler Table
                   4 Safe Exception Handler Count
            004020E4 Guard CF address of check-function pointer
            00000000 Guard CF address of dispatch-function pointer
            00000000 Guard CF function table
                   0 Guard CF function count
            00000100 Guard Flags
                       CF instrumented
                0000 Code Integrity Flags
                0000 Code Integrity Catalog
            00000000 Code Integrity Catalog Offset
            00000000 Code Integrity Reserved
            00000000 Guard CF address taken IAT entry table
                   0 Guard CF address taken IAT entry count
            00000000 Guard CF long jump target table
                   0 Guard CF long jump target count
            00000000 Dynamic value relocation table
            00000000 Hybrid metadata pointer
            00000000 Guard RF address of failure-function
            00000000 Guard RF address of failure-function pointer
            00000000 Dynamic value relocation table offset
                0000 Dynamic value relocation table section
                0000 Reserved2
            00000000 Guard RF address of stack pointer verification function pointer
            00000000 Hot patching table offset
                0000 Reserved3
            00000000 Enclave configuration pointer
            00402290 Volatile metadata pointer
            00000000 Guard EH continuation table
                   0 Guard EH continuation count
            00000000 Guard XFG address of check-function pointer
            00000000 Guard XFG address of dispatch-function pointer
            00000000 Guard XFG address of dispatch-table-function pointer
            004020E8 CastGuard OS determined failure mode
            00000000 Guard memcpy function pointer

    Safe Exception Handler Table

          Address
          --------
           00401C45  __except_handler4
           00401F95
           00401FC0
           00401FE0

    Volatile Metadata

                      18 size
                       2 min version (8002)
                       2 max version (8002)
                000022A8 volatile access RVA table
                      2C volatile access RVA table size
                000022D4 volatile range info table
                      18 volatile range info table size

    Volatile Access RVA Table

          Address
          --------
           0040164B
           0040165B
           00401CAC
           00401CE8
           00401D60
           00401DD5
           00401DD8
           00401DDB
           00401DDE
           00401E23
           00401E2B

    Volatile Info Range Table

          Ranges
          -------
          004012EA - 00401C00
          00401C45 - 00401E54
          00401F08 - 00401F80

  Summary

        1000 .data
        1000 .rdata
        1000 .reloc
        1000 .rsrc
        1000 .text
avivanoff commented 1 year ago

(Also, it looks like you might have forgotten to attach the project.)

Project1.zip

woodruffw commented 1 year ago

Hmm, that does indeed look like a bug: you have a RVA for the SafeSEH handler table, and it looks populated correctly. I'll have some more time to debug tomorrow.

woodruffw commented 1 year ago

cc @yardenshafir since you might have some thoughts as well 🙂

yardenshafir commented 1 year ago

The issue here is that the headers (for some unknown reason) lie about the size of the load config directory. The IMAGE_DATA_DIRECTORY claims that the load config size is 64 (0x40), which means it's too small to contain any SEH fields. But the IMAGE_LOAD_CONFIG_DIRECTORY itself starts with a Size field, and the value there is 192 (0xC0). We only look at the image data directory value, decide that the structure is too small therefore no SEH handlers exist and move on. I don't know why the two values are different but I'll try to change the code to also take the size reported by the load config dir into account and see if that breaks anything.

woodruffw commented 1 year ago

Thanks for triaging! Yeah, it sounds like we should probably pick whichever is larger, so long as it doesn't go OOB.

avivanoff commented 1 year ago

The issue here is that the headers (for some unknown reason) lie about the size of the load config directory.

The reason is known and documented.

woodruffw commented 1 year ago

TIL, thanks for the link. This sounds like something we could potentially fix even further up (in pe-parse), after the initial fix via #1050.