google / syzygy

Syzygy Transformation Toolchain
Apache License 2.0
355 stars 59 forks source link

syzyasan needs updating to support VS 2017 generated binaries #53

Open randomascii opened 7 years ago

randomascii commented 7 years ago

Details are in crbug.com/705133

joshfinley commented 5 years ago

I'm still experiencing issues with the PE parser on Syzygy Instrumenter 0.8.32.0 (190dbfe):

[1018/110110:ERROR:pe_file_parser.cc(1175)] Unknown version of the IMAGE_LOAD_CONFIG_DIRECTORY structure (160 bytes), might be because you're using a new version of the Windows SDK.
[1018/110110:ERROR:pe_file_parser.cc(381)] Failed to parse data directory load config.
[1018/110110:ERROR:decomposer.cc(1084)] Unable to parse PE image.

I've attempted retargeting to older SDK versions but without success.

randomascii commented 5 years ago

It is the C runtime which determines the size of the IMAGE_LOAD_CONFIG_DIRECTORY object. Chrome used to override this by declaring an object of the right size and type, but it no longer does (I removed that a few months ago).

You could use Chrome's hack of defining _load_config_used to specify a custom IMAGE_LOAD_CONFIG_DIRECTORY object. See crrev.com/c/1575870 for the change that removed it.

I did a verbose build of a test project and searched the output for _load_config_used and confirmed that this variable comes from the CRT:

1> Searching C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC\Tools\MSVC\14.23.28105\lib\x64\MSVCRTD.lib: 1> Found _load_config_used 1> Loaded MSVCRTD.lib(loadcfg.obj)

joshfinley commented 5 years ago

Great, thank you!

Adding the custom IMAGE_LOAD_CONFIG_DIRECTORY object and adding the /SAFESEH linker option did the trick.

joshfinley commented 4 years ago

Back again with another issue related to this structure. I ran into the same issue attempting to instrument a DLL. But after adding the custom IMAGE_LOAD_CONFIG_DIRECTORY structure this time the instrumenter is throwing a different error:

[1022/101414:INFO:pe_relinker.cc(57)] Decomposing module: C:\...\ChakraCore.dll
[1022/101419:ERROR:decomposer.cc(1825)] Received data symbol "_load_config_used" that extends past its parent block "Load Config Directory".
[1022/101419:ERROR:pe_relinker.cc(66)] Unable to decompose module: C:\Users\rtadmin\projects\fuzzing\targets\jscript\ChakraCore.dll
[1022/101419:ERROR:instrumenter_with_relinker.cc(124)] Failed to initialize relinker.

The structure looks like this:

Microsoft (R) COFF/PE Dumper Version 14.16.27034.0
Copyright (C) Microsoft Corporation.  All rights reserved.

Dump of file ChakraCore.dll

File Type: DLL

  Section contains the following load config:

            00000048 size
                   0 time date stamp
                0.00 Version
                   0 GlobalFlags Clear
                   0 GlobalFlags Set
                   0 Critical Section Default Timeout
                   0 Decommit Free Block Threshold
              200000 Decommit Total Free Threshold
            00000000 Lock Prefix Table
                   0 Maximum Allocation Size
                   0 Virtual Memory Threshold
                   0 Process Heap Flags
                   0 Process Affinity Mask
                   0 CSD Version
                0000 Dependent Load Flag
            00000000 Edit List
            1055A004 Security Cookie
            1053BD60 Safe Exception Handler Table
                 444 Safe Exception Handler Count

    Safe Exception Handler Table

          Address
          --------
          10446895  __CatchGuardHandler
          1044692F  __TranslatorGuardHandler
          10446FB0  __except_handler4
          10448350
          10448E31
          10467798
          104677C8
          104677F8
          10467830
          10467850
          10467870
          10467890
          104678B0
          104678D8
          10467908
          10467938
          10467968
          ...
          10474470
          10474490
          104744B0
          104744D0
          104744F0
          10474510
          10474530
          10474558
          10474588
          104745B0
          104745D0
          104745F0
          10474618
          104746DA
          10474708
          10474740
          1047475B
          10474776

  Summary

      116000 .data
        1000 .didat
        1000 .mrdata
       E5000 .rdata
       35000 .reloc
       1C000 .rsrc
      474000 .text

Any insight on this would be much appreciated.

randomascii commented 4 years ago

Thanks for editing the output - 1,100 lines was a bit much.

Ultimately syzygy will need to support the new load config structures. I'm not sure what has gone wrong in this case - it sounds like your override of the symbol didn't work, or ??? I'm not sure anybody else is working on this but updating the load-config parser to handle the new fields seems like the right direction to go in.

chhamilton commented 4 years ago

I would strongly suspect we're incorrectly parsing the load config directory. It changes with different compiler/linker version numbers, as can be seen with the parsing code here:

https://github.com/google/syzygy/blob/8164b24ebde9c5649c9a09e88a7fc0b0fcbd1bc5/syzygy/pe/pe_file_parser.cc#L1151

As Bruce points it, it should be updated to handle the new fields that have been added.

sigurasg commented 4 years ago

IIRC there's also some legacy there, as there was a version of the linker or toolchain that emitted the wrong length for the load config symbol, so the parsing code needed to accommodate.

On Tue, Oct 22, 2019 at 2:13 PM Chris Hamilton notifications@github.com wrote:

I would strongly suspect we're incorrectly parsing the load config directory. It changes with different compiler/linker version numbers, as can be seen with the parsing code here:

https://github.com/google/syzygy/blob/8164b24ebde9c5649c9a09e88a7fc0b0fcbd1bc5/syzygy/pe/pe_file_parser.cc#L1151

As Bruce points it, it should be updated to handle the new fields that have been added.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/google/syzygy/issues/53?email_source=notifications&email_token=AALDKF4C5U7YWT7YBR5WYELQP47EXA5CNFSM4DFDNI72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEB6WACA#issuecomment-545087496, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALDKFZZNZWF5MC2IJQLUFLQP47EXANCNFSM4DFDNI7Q .