From 62ac53563c6c4fd43b171309a8188182452597a2 Mon Sep 17 00:00:00 2001 From: EinMByte Date: Thu, 16 Jul 2015 17:39:24 +0200 Subject: [PATCH] Ensure zero-inialization, add TODO update gitignore. --- .gitignore | 1 + I2NPProtocol.cpp | 2 +- I2NPProtocol.h | 2 +- SSUSession.cpp | 26 ++++++++++++-------------- SSUSession.h | 2 ++ TODO | 9 +++++++++ TunnelConfig.h | 4 ++-- aes.h | 8 +++++++- 8 files changed, 35 insertions(+), 19 deletions(-) create mode 100644 TODO diff --git a/.gitignore b/.gitignore index 2da26ecd..156872a9 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ router.keys i2p libi2pd.so netDb +tunnels.cfg # Autotools autom4te.cache diff --git a/I2NPProtocol.cpp b/I2NPProtocol.cpp index d466d205..8ee80000 100644 --- a/I2NPProtocol.cpp +++ b/I2NPProtocol.cpp @@ -353,7 +353,7 @@ namespace i2p } else { - uint8_t clearText[BUILD_REQUEST_RECORD_CLEAR_TEXT_SIZE]; + uint8_t clearText[BUILD_REQUEST_RECORD_CLEAR_TEXT_SIZE] = {}; if (HandleBuildRequestRecords (num, buf + 1, clearText)) { if (clearText[BUILD_REQUEST_RECORD_FLAG_OFFSET] & 0x40) // we are endpoint of outboud tunnel diff --git a/I2NPProtocol.h b/I2NPProtocol.h index 0a4c2fce..bb4c3303 100644 --- a/I2NPProtocol.h +++ b/I2NPProtocol.h @@ -193,7 +193,7 @@ namespace tunnel struct I2NPMessageBuffer: public I2NPMessage { I2NPMessageBuffer () { buf = m_Buffer; maxLen = sz; }; - uint8_t m_Buffer[sz + 16]; + uint8_t m_Buffer[sz + 16] = {}; }; I2NPMessage * NewI2NPMessage (); diff --git a/SSUSession.cpp b/SSUSession.cpp index bfd7a13b..1b89e9ba 100644 --- a/SSUSession.cpp +++ b/SSUSession.cpp @@ -275,7 +275,7 @@ namespace transport return; } - uint8_t buf[320 + 18]; // 304 bytes for ipv4, 320 for ipv6 + uint8_t buf[320 + 18] = {}; // 304 bytes for ipv4, 320 for ipv6, all set to 0 uint8_t * payload = buf + sizeof (SSUHeader); memcpy (payload, m_DHKeysPair->publicKey, 256); // x bool isV4 = m_RemoteEndpoint.address ().is_v4 (); @@ -306,7 +306,7 @@ namespace transport return; } - uint8_t buf[96 + 18]; + uint8_t buf[96 + 18] = {}; uint8_t * payload = buf + sizeof (SSUHeader); htobe32buf (payload, iTag); payload += 4; @@ -344,7 +344,7 @@ namespace transport SignedData s; // x,y, remote IP, remote port, our IP, our port, relayTag, signed on time s.Insert (x, 256); // x - uint8_t buf[384 + 18]; + uint8_t buf[384 + 18] = {}; uint8_t * payload = buf + sizeof (SSUHeader); memcpy (payload, m_DHKeysPair->publicKey, 256); s.Insert (payload, 256); // y @@ -408,7 +408,7 @@ namespace transport void SSUSession::SendSessionConfirmed (const uint8_t * y, const uint8_t * ourAddress, size_t ourAddressLen) { - uint8_t buf[512 + 18]; + uint8_t buf[512 + 18] = {}; uint8_t * payload = buf + sizeof (SSUHeader); *payload = 1; // 1 fragment payload++; // info @@ -475,7 +475,7 @@ namespace transport void SSUSession::SendRelayResponse (uint32_t nonce, const boost::asio::ip::udp::endpoint& from, const uint8_t * introKey, const boost::asio::ip::udp::endpoint& to) { - uint8_t buf[80 + 18]; // 64 Alice's ipv4 and 80 Alice's ipv6 + uint8_t buf[80 + 18] = {}; // 64 Alice's ipv4 and 80 Alice's ipv6 uint8_t * payload = buf + sizeof (SSUHeader); // Charlie's address always v4 if (!to.address ().is_v4 ()) @@ -536,7 +536,7 @@ namespace transport LogPrint (eLogError, "Alice's IP must be v4"); return; } - uint8_t buf[48 + 18]; + uint8_t buf[48 + 18] = {}; uint8_t * payload = buf + sizeof (SSUHeader); *payload = 4; payload++; // size @@ -616,9 +616,7 @@ namespace transport htobe32buf (&(header->time), i2p::util::GetSecondsSinceEpoch ()); uint8_t * encrypted = &header->flag; uint16_t encryptedLen = len - (encrypted - buf); - i2p::crypto::CBCEncryption encryption; - encryption.SetKey (aesKey); - encryption.SetIV (iv); + i2p::crypto::CBCEncryption encryption(aesKey, iv); encryption.Encrypt (encrypted, encryptedLen, encrypted); // assume actual buffer size is 18 (16 + 2) bytes more memcpy (buf + len, iv, 16); @@ -960,8 +958,7 @@ namespace transport // toAddress is true for Alice<->Chalie communications only // sendAddress is false if message comes from Alice { - uint8_t buf[80 + 18]; - uint8_t iv[16]; + uint8_t buf[80 + 18] = {}; uint8_t * payload = buf + sizeof (SSUHeader); htobe32buf (payload, nonce); payload += 4; // nonce @@ -995,6 +992,7 @@ namespace transport // send CryptoPP::RandomNumberGenerator& rnd = i2p::context.GetRandomNumberGenerator (); + uint8_t iv[16]; rnd.GenerateBlock (iv, 16); // random iv if (toAddress) { @@ -1032,7 +1030,7 @@ namespace transport { if (m_State == eSessionStateEstablished) { - uint8_t buf[48 + 18]; + uint8_t buf[48 + 18] = {}; uint8_t * payload = buf + sizeof (SSUHeader); *payload = 0; // flags payload++; @@ -1049,7 +1047,7 @@ namespace transport { if (m_IsSessionKey) { - uint8_t buf[48 + 18]; + uint8_t buf[48 + 18] = {}; // encrypt message with session key FillHeaderAndEncrypt (PAYLOAD_TYPE_SESSION_DESTROYED, buf, 48); try @@ -1066,7 +1064,7 @@ namespace transport void SSUSession::Send (uint8_t type, const uint8_t * payload, size_t len) { - uint8_t buf[SSU_MTU_V4 + 18]; + uint8_t buf[SSU_MTU_V4 + 18] = {}; size_t msgSize = len + sizeof (SSUHeader); size_t paddingSize = msgSize & 0x0F; // %16 if (paddingSize > 0) msgSize += (16 - paddingSize); diff --git a/SSUSession.h b/SSUSession.h index 6c222185..0cbe3e52 100644 --- a/SSUSession.h +++ b/SSUSession.h @@ -15,6 +15,8 @@ namespace i2p namespace transport { #pragma pack(1) + // Warning: do not change the order of these variables + // (or fix the unsafe casts in SSU.h) struct SSUHeader { uint8_t mac[16]; diff --git a/TODO b/TODO new file mode 100644 index 00000000..0ad96b98 --- /dev/null +++ b/TODO @@ -0,0 +1,9 @@ +Short-term refactoring: + - SSUSession:637, SSUSession:635 get rid of casting to SSUHeader + +Long-term refactoring: + - Rely on a library for TLS and SSL. + +Additions: + - Write tests. + - Add documentation. diff --git a/TunnelConfig.h b/TunnelConfig.h index d9827fe8..697005fd 100644 --- a/TunnelConfig.h +++ b/TunnelConfig.h @@ -86,8 +86,8 @@ namespace tunnel void CreateBuildRequestRecord (uint8_t * record, uint32_t replyMsgID) const { - uint8_t clearText[BUILD_REQUEST_RECORD_CLEAR_TEXT_SIZE]; - htobe32buf (clearText + BUILD_REQUEST_RECORD_RECEIVE_TUNNEL_OFFSET, tunnelID); + uint8_t clearText[BUILD_REQUEST_RECORD_CLEAR_TEXT_SIZE] = {}; + htobe32buf (clearText + BUILD_REQUEST_RECORD_RECEIVE_TUNNEL_OFFSET, tunnelID); memcpy (clearText + BUILD_REQUEST_RECORD_OUR_IDENT_OFFSET, router->GetIdentHash (), 32); htobe32buf (clearText + BUILD_REQUEST_RECORD_NEXT_TUNNEL_OFFSET, nextTunnelID); memcpy (clearText + BUILD_REQUEST_RECORD_NEXT_IDENT_OFFSET, nextRouter->GetIdentHash (), 32); diff --git a/aes.h b/aes.h index 84ca1f17..56dd97b9 100644 --- a/aes.h +++ b/aes.h @@ -140,7 +140,13 @@ namespace crypto { public: - CBCEncryption () { memset (m_LastBlock.buf, 0, 16); }; + CBCEncryption () { memset (m_LastBlock.buf, 0, 16); }; + CBCEncryption(const AESKey& key, const uint8_t* iv) + : CBCEncryption() + { + SetKey(key); + SetIV(iv); + }; void SetKey (const AESKey& key) { m_ECBEncryption.SetKey (key); }; // 32 bytes void SetIV (const uint8_t * iv) { memcpy (m_LastBlock.buf, iv, 16); }; // 16 bytes