fileinfo crashes when verifying digital signature of a PE file

[s3rvac]

fileinfo crashes when verifying the digital signature of the attached PE file.

Input

Run

$ retdec-fileinfo FILE

where FILE is:

• DA9AC36A6A69CBE79D266AE695906296F8AABDE7713624D29553BC22508D9AC3
• 65BB4991028F627C5B832C5E9189118FF656F71481931BB98EB9F211FA4F6B5F

Output

Segmentation fault

Expected output

fileinfo does not crash when analyzing the file.

Output from valgrind

Invalid read of size 8
   at 0x29CDBF: retdec::fileformat::PeFormat::verifySignature(pkcs7_st*) (pe_format.cpp:1220)
   by 0x29BF0C: retdec::fileformat::PeFormat::loadCertificates() (pe_format.cpp:1026)
   by 0x298337: retdec::fileformat::PeFormat::initStructures() (pe_format.cpp:385)
   by 0x297B81: retdec::fileformat::PeFormat::PeFormat(...) (pe_format.cpp:296)
   by 0x20C444: fileinfo::PeWrapper::PeWrapper(...) (pe_wrapper.cpp:96)
   by 0x194903: void __gnu_cxx::new_allocator<...>::construct<...>(...) (new_allocator.h:136)
   by 0x1947BD: void std::allocator_traits<...>::construct<...>(...) (alloc_traits.h:475)
   by 0x194600: std::_Sp_counted_ptr_inplace<...>::_Sp_counted_ptr_inplace<...>(...) (shared_ptr_base.h:526)
   by 0x194356: std::__shared_count<(...)2>::__shared_count<...>(...) (shared_ptr_base.h:637)
   by 0x194193: std::__shared_ptr<...>::__shared_ptr<...>(...) (shared_ptr_base.h:1295)
   by 0x194068: std::shared_ptr<...>::shared_ptr<...>(...) (shared_ptr.h:344)
   by 0x193EF5: std::shared_ptr<...> std::allocate_shared<...>(...) (shared_ptr.h:691)
 Address 0x8 is not stack'd, malloc'd or (recently) free'd

Notes

• This is a different error than the one in fileinfo crashes when verifying digital signature of attached PE files #87 (see the traceback from valgrind above; here, it is most probably a null-pointer access).

Configuration

• Commit: a24aabb (current master)
• 64b Arch Linux, GCC 7.3.0, Debug build of RetDec

[s3rvac]

There is indeed a null pointer access. The PeFormat::verifySignature() function in src/fileformat/file_format/pe/pe_format.cpp contains the following line:

1220     auto contentInfoPtr = p7->d.sign->contents->d.other->value.sequence->data;

For the attached PE file, the following variable is the null pointer:

p7->d.sign->contents->d.other

Thus, when accessing it (d.other->value), fileinfo crashes on a null-pointer dereference.

BTW, to be honest, the following two lines seem to be very error-prone in regards to possible null-pointer dereferences:

1220     auto contentInfoPtr = p7->d.sign->contents->d.other->value.sequence->data;
1221     auto contentInfoLen = p7->d.sign->contents->d.other->value.sequence->length;

I am not sure what guarantees OpenSSL provides, but we should probably add more checks there to make the code more robust.

[metthal]

Fixed in ec88142. PKCS7 strucutre in these files did not contain any Microsoft Code Signing structure. I added check for this edge case.