Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fileinfo crashes when verifying digital signature of attached PE files #87

Closed
s3rvac opened this issue Jan 14, 2018 · 1 comment
Closed

fileinfo crashes when verifying digital signature of attached PE files #87

s3rvac opened this issue Jan 14, 2018 · 1 comment
Labels
bug C-fileformat P-run

Comments

@s3rvac
Copy link
Member

s3rvac commented Jan 14, 2018 •
edited
Loading

fileinfo crashes when verifying digital signature of attached PE files.

Input

$ fileinfo FILE

where FILE is any of the PE files below:

Output

Segmentation fault

Expected output

fileinfo does not crash when analyzing the files above.

Output from valgrind

Invalid read of size 8
   at 0x2C1FDB: fileformat::PeFormat::verifySignature(pkcs7_st*) (pe_format.cpp:1215)
   by 0x2C0F07: fileformat::PeFormat::loadCertificates() (pe_format.cpp:977)
   by 0x2BD577: fileformat::PeFormat::initStructures() (pe_format.cpp:384)
   by 0x2BCDC1: fileformat::PeFormat::PeFormat(...) (pe_format.cpp:295)
   by 0x23AD36: fileinfo::PeWrapper::PeWrapper(...) (pe_wrapper.cpp:96)
   by 0x1C3181: void __gnu_cxx::new_allocator<...>::construct<...>(...) (new_allocator.h:136)
   by 0x1C303B: void std::allocator_traits<...>::construct<...>(...) (alloc_traits.h:475)
   by 0x1C2E7E: std::_Sp_counted_ptr_inplace<...>::_Sp_counted_ptr_inplace<...>(...) (shared_ptr_base.h:526)
   by 0x1C2BC6: std::__shared_count<...>::__shared_count<...>(...) (shared_ptr_base.h:637)
   by 0x1C2A11: std::__shared_ptr<...>::__shared_ptr<...>(...) (shared_ptr_base.h:1295)
   by 0x1C28D8: std::shared_ptr<...>::shared_ptr<...>(...) (shared_ptr.h:344)
   by 0x1C2765: std::shared_ptr<...> std::allocate_shared<...>(...) (shared_ptr.h:691)
 Address 0x6bc1bb8 is 24 bytes after a block of size 16 in arena "client"

Configuration

  • Commit: 8c4b23d (current master)
  • 64b Arch Linux, GCC 7.2.1, Debug build of RetDec
  • Also fails on 64b Debian Jessie, GCC 4.9.2
s3rvac added a commit that referenced this issue Mar 17, 2018
@s3rvac
fileformat: Verify digital signature after we have a signer/counter-s…
168d23e 
…igner (#87).

In #87, a fileinfo crash is reported when verifying the digital signature of
attached PE files. What all the attached files have in common is that we are
unable to find a signer or counter-signer for them and p7->length is 0. As the
following comment in pe_format.cpp suggests, there is no point of continuing in
such a case:

    // If we have no signer and countersigner, there must be something really bad
    if(!signerCert && !counterSignerCert)
    {
        BIO_free(bio);
        return;
    }

Thus, move the signature verification AFTER the check that we have found a
signer or a counter-signer. This fixes the signature-verifying crashes for all
the files attached to #87.
This was referenced Mar 17, 2018
Merged
Closed
s3rvac added a commit to avast/retdec-regression-tests that referenced this issue Mar 17, 2018
@s3rvac
tools/fileinfo: Add a test for avast/retdec#87.
ce189d6
@s3rvac s3rvac mentioned this issue Mar 17, 2018
Merged
metthal added a commit to avast/retdec-regression-tests that referenced this issue Mar 17, 2018
@metthal
Merge pull request #2 from avast-tl/test-for-fix-fileinfo-crashing-wh…
2a805e4 
…en-verifying-signatures

Add a test for avast/retdec#87
@s3rvac
Copy link
Member Author

s3rvac commented Mar 18, 2018

Fixed by PR #249 (commit 168d23e). A regression test was added in avast/retdec-regression-tests@ce189d6.

@s3rvac s3rvac closed this as completed Mar 18, 2018
@s3rvac s3rvac mentioned this issue Mar 19, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug C-fileformat P-run
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@s3rvac