Merge bitcoin/bitcoin#28695: net: Sanity check private keys received from SAM proxy

5cf4d266d9 [test] Test i2p private key constraints (Vasil Dimov)
cf70a8d565 [net] Check i2p private key constraints (dergoegge)

Pull request description:

  Not sanity checking can lead to crashes or worse:

  ```
  ==1715589==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6140000055c2 at pc 0x5622ed66e7ad bp 0x7ffee547a2c0 sp 0x7ffee547a2b8
  READ of size 2 at 0x6140000055c2 thread T0 (b-test)
      #0 0x5622ed66e7ac in memcpy include/bits/string_fortified.h:29:10
      #1 0x5622ed66e7ac in i2p::sam::Session::MyDestination() const src/i2p.cpp:362:5
      #2 0x5622ed662e46 in i2p::sam::Session::CreateIfNotCreatedAlready() src/i2p.cpp:414:40
      #3 0x5622ed6619f2 in i2p::sam::Session::Listen(i2p::Connection&) src/i2p.cpp:143:9
  ```

ACKs for top commit:
  maflcko:
    code lgtm ACK 5cf4d266d9
  stickies-v:
    re-ACK 5cf4d266d9
  vasild:
    ACK 5cf4d266d9

Tree-SHA512: 3de3bd396538fa619de67957b9c8a58011ab911f0f51097c387e730c13908278b7322aa3357051fb245a20b15bef34b0e9fadcb1eff8ad751139d2aa634c78ad
This commit is contained in:
fanquake
2023-10-30 14:26:25 +01:00
2 changed files with 59 additions and 0 deletions

View File

@@ -384,11 +384,26 @@ Binary Session::MyDestination() const
static constexpr size_t CERT_LEN_POS = 385;
uint16_t cert_len;
if (m_private_key.size() < CERT_LEN_POS + sizeof(cert_len)) {
throw std::runtime_error(strprintf("The private key is too short (%d < %d)",
m_private_key.size(),
CERT_LEN_POS + sizeof(cert_len)));
}
memcpy(&cert_len, &m_private_key.at(CERT_LEN_POS), sizeof(cert_len));
cert_len = be16toh(cert_len);
const size_t dest_len = DEST_LEN_BASE + cert_len;
if (dest_len > m_private_key.size()) {
throw std::runtime_error(strprintf("Certificate length (%d) designates that the private key should "
"be %d bytes, but it is only %d bytes",
cert_len,
dest_len,
m_private_key.size()));
}
return Binary{m_private_key.begin(), m_private_key.begin() + dest_len};
}