fileinfo crashes when computing SHA-1 hash of PE digital signature

[s3rvac]

fileinfo crashes when computing the SHA-1 hash of a digital signature of the attached PE file.

Input

Run

retdec-fileinfo FILE

where FILE is:

• 34722C26B5557979DE5B4DCAE01DD4D0CD1DC99AF78656D7DA93D3B6BB907C9A

Output

Segmentation fault

Expected output

fileinfo does not crash during the analysis.

Output from valgrind

Invalid read of size 8
   at 0x5A7F8AB: SHA1_Update (in /usr/lib/libcrypto.so.1.1)
   by 0x64831B: retdec::crypto::HashContext::addData(unsigned char const*, unsigned long) (hash_context.cpp:71)
   by 0x29DB39: retdec::fileformat::PeFormat::calculateDigest[abi:cxx11](retdec::crypto::HashAlgorithm) const (pe_format.cpp:1340)
   by 0x29D1C6: retdec::fileformat::PeFormat::verifySignature(pkcs7_st*) (pe_format.cpp:1260)
   by 0x29BCD5: retdec::fileformat::PeFormat::loadCertificates() (pe_format.cpp:978)
   by 0x298337: retdec::fileformat::PeFormat::initStructures() (pe_format.cpp:385)
   by 0x297B81: retdec::fileformat::PeFormat::PeFormat(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, retdec::fileformat::LoadFlags) (pe_format.cpp:296)
   by 0x20C444: fileinfo::PeWrapper::PeWrapper(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, retdec::fileformat::LoadFlags) (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<...>::__shared_count<...>(...) (shared_ptr_base.h:637)
 Address 0x107160988 is not stack'd, malloc'd or (recently) free'd

Notes

This issue may be related to #87, but there are several differences:

• In #87, the crash happens right at the beginning of verifySignature(), but in this case, it crashes when verifying the digest by computing the SHA-1 hash.
• In #87, we are unable to find a signer or counter-signer, but in this case, we are able to find both signer and counter-signer. Moreover, if I disable signature verification by commenting it out, retdec-fileinfo --verbose is able to parse the certificate table in this case:

  
  Certificate table
  -----------------
  Number of certificates          : 3
  Signer certificate index        : 0
  Counter-signer certificate index: 1

Certificate #0 Subject name : Systweak Inc ...

Certificate #1 Subject name : VeriSign Time Stamping Services Signer - G2 ..

Certificate #2 Subject name : VeriSign Time Stamping Services CA ...

* The fix proposed in #249 fixes #87 but is insufficient to fix the present issue.

## Configuration

* Commit: 8cc759b70f2ccda3eac2b4d710043fe04b16fc44 (current `master`)
* 64b Arch Linux, GCC 7.3.0, Debug build of RetDec
* OpenSSL 1.1.0.g (system version)

[s3rvac]

I looked into this for a bit and here is what I found. PeFormat::calculateDigest() computes the hash by iterating over all ranges returned by getDigestRanges() and adding the data into the hashing context:

1334     auto digestRanges = getDigestRanges();
1335     for (const auto& range : digestRanges)
1336     {
1337         const std::uint8_t* data = std::get<0>(range);
1338         std::size_t size = std::get<1>(range);
1339
1340         if (!hashCtx.addData(data, size))
1341             return {};
1342     }

For the attached file, it reports the following ranges (data and size from the above code):

0x7f3b8231b010 352
0x7f3b8231b174 60
0x7f3b8231b1b8 1566296
0x7f3c8247fff8 18446744069414693120

The last range is clearly off and should not be there. In more detail, if you subtract 0x7f3c8247fff8 (the beginning of the last alleged range) and 0x7f3b8231b010 (the beginning of the loaded file data in memory), you get 4296429544, which is approx. 4 GB. The file has 1.5 MB, which is nowhere near 4 GB.

This fact and the size of the last range suggests that there is an overflow somewhere inside getDigestRanges(). And there is indeed an overflow. The last range is added in the following code:

1316     // Finish off the data if the last offset didn't end at the end of of all data
1317     if (lastOffset != bytes.size())
1318         result.emplace_back(bytes.data() + lastOffset, bytes.size() - lastOffset);

In our case, lastOffset reports 4296429544 and bytes.size() is 1571048. The actual reason why lastOffset is that big is that formatParser->getSecurityDirSize() returns 4294862824:

1287     std::size_t secDirSize = formatParser->getSecurityDirSize();

The secDirSize variable is then used to compute the end of the last range:

1293     std::vector<std::pair<...>> offsets = { [..], std::make_pair(secDirOffset, secDirSize) };

The size of the security directory is obtained from pelib in include/pelib/PeHeader.h:

1911     /**
1912     * Returns the size of the current file's security directory.
1913     * @return The size of the Security directory.
1914     **/
1915     template<int x>
1916     dword PeHeaderT<x>::getIddSecuritySize() const
1917     {
1918         return m_inthHeader.dataDirectories[PELIB_IMAGE_DIRECTORY_ENTRY_SECURITY].Size;
1919     }
1920

This function indeed returns 4294862824. So, either there is a bug in pelib, or the binary contains an invalid value. However, it seems to be the latter as pefile also reports this value:

[IMAGE_DIRECTORY_ENTRY_SECURITY]
0x1A0      0x0   VirtualAddress:                0x17E800
0x1A4      0x4   Size:                          0xFFFE67E8

[metthal]

Fixed in 714d69c824ec99585fd15bb005030c86c3466bc6.