From 84f7966a0b7957d3e4e49aa1d8748c19cb94b6b8 Mon Sep 17 00:00:00 2001 From: "Francisco Blas (klondike) Izquierdo Riera" Date: Wed, 31 Dec 2014 15:14:53 +0100 Subject: [PATCH] Fix even more alignment problems --- Destination.cpp | 18 +++++++------ Garlic.cpp | 2 +- I2NPProtocol.cpp | 67 ++++++++++++++++++++++++++++-------------------- I2NPProtocol.h | 2 ++ I2PEndian.h | 26 +++++++++---------- LeaseSet.cpp | 15 ++++++----- NetDb.cpp | 12 +++++---- SSUSession.cpp | 11 ++++++-- Tunnel.cpp | 9 ++++--- 9 files changed, 96 insertions(+), 66 deletions(-) diff --git a/Destination.cpp b/Destination.cpp index 5a338a60..307e803a 100644 --- a/Destination.cpp +++ b/Destination.cpp @@ -188,6 +188,7 @@ namespace client void ClientDestination::HandleI2NPMessage (const uint8_t * buf, size_t len, i2p::tunnel::InboundTunnel * from) { + //TODO: since we are accessing a uint8_t this is unlikely to crash due to alignment but should be improved I2NPHeader * header = (I2NPHeader *)buf; switch (header->typeID) { @@ -207,14 +208,15 @@ namespace client void ClientDestination::HandleDatabaseStoreMessage (const uint8_t * buf, size_t len) { - I2NPDatabaseStoreMsg * msg = (I2NPDatabaseStoreMsg *)buf; + I2NPDatabaseStoreMsg msg; + memcpy(&msg,buf,sizeof (I2NPDatabaseStoreMsg)); size_t offset = sizeof (I2NPDatabaseStoreMsg); - if (msg->replyToken) // TODO: + if (msg.replyToken) // TODO: offset += 36; - if (msg->type == 1) // LeaseSet + if (msg.type == 1) // LeaseSet { LogPrint (eLogDebug, "Remote LeaseSet"); - auto it = m_RemoteLeaseSets.find (msg->key); + auto it = m_RemoteLeaseSets.find (msg.key); if (it != m_RemoteLeaseSets.end ()) { it->second->Update (buf + offset, len - offset); @@ -223,13 +225,13 @@ namespace client else { LogPrint (eLogDebug, "New remote LeaseSet added"); - m_RemoteLeaseSets[msg->key] = new i2p::data::LeaseSet (buf + offset, len - offset); + m_RemoteLeaseSets[msg.key] = new i2p::data::LeaseSet (buf + offset, len - offset); } } else - LogPrint (eLogError, "Unexpected client's DatabaseStore type ", msg->type, ". Dropped"); + LogPrint (eLogError, "Unexpected client's DatabaseStore type ", msg.type, ". Dropped"); - auto it1 = m_LeaseSetRequests.find (msg->key); + auto it1 = m_LeaseSetRequests.find (msg.key); if (it1 != m_LeaseSetRequests.end ()) { it1->second->requestTimeoutTimer.cancel (); @@ -286,7 +288,7 @@ namespace client void ClientDestination::HandleDeliveryStatusMessage (I2NPMessage * msg) { I2NPDeliveryStatusMsg * deliveryStatus = (I2NPDeliveryStatusMsg *)msg->GetPayload (); - uint32_t msgID = be32toh (deliveryStatus->msgID); + uint32_t msgID = bufbe32toh (&(deliveryStatus->msgID)); if (msgID == m_PublishReplyToken) { LogPrint (eLogDebug, "Publishing confirmed"); diff --git a/Garlic.cpp b/Garlic.cpp index d312b467..ff773978 100644 --- a/Garlic.cpp +++ b/Garlic.cpp @@ -168,7 +168,7 @@ namespace garlic buf[blockSize] = 0; // flag blockSize++; size_t len = CreateGarlicPayload (buf + blockSize, msg, newTags); - *payloadSize = htobe32 (len); + htobe32buf (payloadSize, len); CryptoPP::SHA256().CalculateDigest(payloadHash, buf + blockSize, len); blockSize += len; size_t rem = blockSize % 16; diff --git a/I2NPProtocol.cpp b/I2NPProtocol.cpp index c2ae6d3b..8b5b7321 100644 --- a/I2NPProtocol.cpp +++ b/I2NPProtocol.cpp @@ -238,11 +238,12 @@ namespace i2p router = &context.GetRouterInfo (); I2NPMessage * m = NewI2NPShortMessage (); - I2NPDatabaseStoreMsg * msg = (I2NPDatabaseStoreMsg *)m->GetPayload (); + I2NPDatabaseStoreMsg msg; - memcpy (msg->key, router->GetIdentHash (), 32); - msg->type = 0; - msg->replyToken = 0; + memcpy (msg.key, router->GetIdentHash (), 32); + msg.type = 0; + msg.replyToken = 0; + memcpy(m->GetPayload (),&msg,sizeof(I2NPDatabaseStoreMsg)); CryptoPP::Gzip compressor; compressor.Put (router->GetBuffer (), router->GetBufferLen ()); @@ -264,10 +265,10 @@ namespace i2p if (!leaseSet) return nullptr; I2NPMessage * m = NewI2NPShortMessage (); uint8_t * payload = m->GetPayload (); - I2NPDatabaseStoreMsg * msg = (I2NPDatabaseStoreMsg *)payload; - memcpy (msg->key, leaseSet->GetIdentHash (), 32); - msg->type = 1; // LeaseSet - msg->replyToken = htobe32 (replyToken); + I2NPDatabaseStoreMsg msg; + memcpy (msg.key, leaseSet->GetIdentHash (), 32); + msg.type = 1; // LeaseSet + msg.replyToken = htobe32 (replyToken); size_t size = sizeof (I2NPDatabaseStoreMsg); if (replyToken) { @@ -280,8 +281,9 @@ namespace i2p size += 32; // reply tunnel gateway } else - msg->replyToken = 0; + msg.replyToken = 0; } + memcpy(payload,&msg,sizeof (I2NPDatabaseStoreMsg)); memcpy (payload + size, leaseSet->GetBuffer (), leaseSet->GetBufferLen ()); size += leaseSet->GetBufferLen (); m->len += size; @@ -331,6 +333,7 @@ namespace i2p i2p::crypto::ElGamalDecrypt (i2p::context.GetEncryptionPrivateKey (), records[i].encrypted, (uint8_t *)&clearText); // replace record to reply + //HACK: Ugly but since all is uint8_t should work I2NPBuildResponseRecord * reply = (I2NPBuildResponseRecord *)(records + i); if (i2p::context.AcceptsTunnels ()) { @@ -386,6 +389,7 @@ namespace i2p } else { + //HACK: Ugly but since all is uint8_t should work I2NPBuildRequestRecordElGamalEncrypted * records = (I2NPBuildRequestRecordElGamalEncrypted *)(buf+1); I2NPBuildRequestRecordClearText clearText; if (HandleBuildRequestRecords (num, records, clearText)) @@ -408,6 +412,7 @@ namespace i2p void HandleTunnelBuildMsg (uint8_t * buf, size_t len) { I2NPBuildRequestRecordClearText clearText; + //HACK: Ugly but since all is uint8_t should work if (HandleBuildRequestRecords (NUM_TUNNEL_BUILD_RECORDS, (I2NPBuildRequestRecordElGamalEncrypted *)buf, clearText)) { if (clearText.flag & 0x40) // we are endpoint of outbound tunnel @@ -470,9 +475,10 @@ namespace i2p I2NPMessage * CreateTunnelGatewayMsg (uint32_t tunnelID, const uint8_t * buf, size_t len) { I2NPMessage * msg = NewI2NPMessage (len); - TunnelGatewayHeader * header = (TunnelGatewayHeader *)msg->GetPayload (); - header->tunnelID = htobe32 (tunnelID); - header->length = htobe16 (len); + TunnelGatewayHeader header; + header.tunnelID = htobe32 (tunnelID); + header.length = htobe16 (len); + memcpy (msg->GetPayload (), &header, sizeof (TunnelGatewayHeader)); memcpy (msg->GetPayload () + sizeof (TunnelGatewayHeader), buf, len); msg->len += sizeof (TunnelGatewayHeader) + len; FillI2NPMessageHeader (msg, eI2NPTunnelGateway); @@ -484,10 +490,11 @@ namespace i2p if (msg->offset >= sizeof (I2NPHeader) + sizeof (TunnelGatewayHeader)) { // message is capable to be used without copying - TunnelGatewayHeader * header = (TunnelGatewayHeader *)(msg->GetBuffer () - sizeof (TunnelGatewayHeader)); - header->tunnelID = htobe32 (tunnelID); + TunnelGatewayHeader header; + header.tunnelID = htobe32 (tunnelID); int len = msg->GetLength (); - header->length = htobe16 (len); + header.length = htobe16 (len); + memcpy (msg->GetBuffer () - sizeof (TunnelGatewayHeader), &header, sizeof (TunnelGatewayHeader)); msg->offset -= (sizeof (I2NPHeader) + sizeof (TunnelGatewayHeader)); msg->len = msg->offset + sizeof (I2NPHeader) + sizeof (TunnelGatewayHeader) +len; FillI2NPMessageHeader (msg, eI2NPTunnelGateway); @@ -513,18 +520,21 @@ namespace i2p FillI2NPMessageHeader (msg, msgType, replyMsgID); // create content message len = msg->GetLength (); msg->offset -= gatewayMsgOffset; - TunnelGatewayHeader * header = (TunnelGatewayHeader *)msg->GetPayload (); - header->tunnelID = htobe32 (tunnelID); - header->length = htobe16 (len); + TunnelGatewayHeader header; + header.tunnelID = htobe32 (tunnelID); + header.length = htobe16 (len); + memcpy (msg->GetPayload (), &header, sizeof (TunnelGatewayHeader)); FillI2NPMessageHeader (msg, eI2NPTunnelGateway); // gateway message return msg; } void HandleTunnelGatewayMsg (I2NPMessage * msg) { - TunnelGatewayHeader * header = (TunnelGatewayHeader *)msg->GetPayload (); - uint32_t tunnelID = be32toh(header->tunnelID); - uint16_t len = be16toh(header->length); + TunnelGatewayHeader header; + uint32_t tunnelID = be32toh(header.tunnelID); + uint16_t len = be16toh(header.length); + memcpy (msg->GetPayload (), &header, sizeof (TunnelGatewayHeader)); + // we make payload as new I2NP message to send msg->offset += sizeof (I2NPHeader) + sizeof (TunnelGatewayHeader); msg->len = msg->offset + len; @@ -551,18 +561,19 @@ namespace i2p size_t GetI2NPMessageLength (const uint8_t * msg) { I2NPHeader * header = (I2NPHeader *)msg; - return be16toh (header->size) + sizeof (I2NPHeader); + return bufbe16toh (&(header->size)) + sizeof (I2NPHeader); } void HandleI2NPMessage (uint8_t * msg, size_t len) { - I2NPHeader * header = (I2NPHeader *)msg; - uint32_t msgID = be32toh (header->msgID); - LogPrint ("I2NP msg received len=", len,", type=", (int)header->typeID, ", msgID=", (unsigned int)msgID); + I2NPHeader header; + memcpy (&header,msg,sizeof(I2NPHeader)); + uint32_t msgID = be32toh (header.msgID); + LogPrint ("I2NP msg received len=", len,", type=", (int)header.typeID, ", msgID=", (unsigned int)msgID); uint8_t * buf = msg + sizeof (I2NPHeader); - int size = be16toh (header->size); - switch (header->typeID) + int size = be16toh (header.size); + switch (header.typeID) { case eI2NPVariableTunnelBuild: LogPrint ("VariableTunnelBuild"); @@ -581,7 +592,7 @@ namespace i2p // TODO: break; default: - LogPrint ("Unexpected message ", (int)header->typeID); + LogPrint ("Unexpected message ", (int)header.typeID); } } diff --git a/I2NPProtocol.h b/I2NPProtocol.h index 9d3fa79b..a9665508 100644 --- a/I2NPProtocol.h +++ b/I2NPProtocol.h @@ -114,6 +114,7 @@ namespace tunnel I2NPMessage (): buf (nullptr),len (sizeof (I2NPHeader) + 2), offset(2), maxLen (0), from (nullptr) {}; // reserve 2 bytes for NTCP header + //TODO: This is VERY likely to cause alignment problems and I'm not willing to fix it now I2NPHeader * GetHeader () { return (I2NPHeader *)GetBuffer (); }; uint8_t * GetPayload () { return GetBuffer () + sizeof(I2NPHeader); }; uint8_t * GetBuffer () { return buf + offset; }; @@ -138,6 +139,7 @@ namespace tunnel } // for SSU only + //TODO: This is VERY likely to cause alignment problems and I'm not willing to fix it now uint8_t * GetSSUHeader () { return buf + offset + sizeof(I2NPHeader) - sizeof(I2NPHeaderShort); }; void FromSSU (uint32_t msgID) // we have received SSU message and convert it to regular { diff --git a/I2PEndian.h b/I2PEndian.h index d8be0f82..687dbc63 100644 --- a/I2PEndian.h +++ b/I2PEndian.h @@ -1,7 +1,7 @@ #ifndef I2PENDIAN_H__ #define I2PENDIAN_H__ #include -#include +#include #if defined(__linux__) || defined(__FreeBSD_kernel__) #include @@ -47,68 +47,68 @@ uint64_t be64toh(uint64_t big64); #endif -inline uint16_t buf16toh(const uint8_t *buf) +inline uint16_t buf16toh(const void *buf) { uint16_t b16; memcpy(&b16, buf, sizeof(uint16_t)); return b16; } -inline uint32_t buf32toh(const uint8_t *buf) +inline uint32_t buf32toh(const void *buf) { uint32_t b32; memcpy(&b32, buf, sizeof(uint32_t)); return b32; } -inline uint64_t buf64toh(const uint8_t *buf) +inline uint64_t buf64toh(const void *buf) { uint64_t b64; memcpy(&b64, buf, sizeof(uint64_t)); return b64; } -inline uint16_t bufbe16toh(const uint8_t *buf) +inline uint16_t bufbe16toh(const void *buf) { return be16toh(buf16toh(buf)); } -inline uint32_t bufbe32toh(const uint8_t *buf) +inline uint32_t bufbe32toh(const void *buf) { return be32toh(buf32toh(buf)); } -inline uint64_t bufbe64toh(const uint8_t *buf) +inline uint64_t bufbe64toh(const void *buf) { return be64toh(buf64toh(buf)); } -inline void htobuf16(uint8_t *buf, uint16_t b16) +inline void htobuf16(void *buf, uint16_t b16) { memcpy(buf, &b16, sizeof(uint16_t)); } -inline void htobuf32(uint8_t *buf, uint32_t b32) +inline void htobuf32(void *buf, uint32_t b32) { memcpy(buf, &b32, sizeof(uint32_t)); } -inline void htobuf64(uint8_t *buf, uint64_t b64) +inline void htobuf64(void *buf, uint64_t b64) { memcpy(buf, &b64, sizeof(uint64_t)); } -inline void htobe16buf(uint8_t *buf, uint16_t big16) +inline void htobe16buf(void *buf, uint16_t big16) { htobuf16(buf, htobe16(big16)); } -inline void htobe32buf(uint8_t *buf, uint32_t big32) +inline void htobe32buf(void *buf, uint32_t big32) { htobuf32(buf, htobe32(big32)); } -inline void htobe64buf(uint8_t *buf, uint64_t big64) +inline void htobe64buf(void *buf, uint64_t big64) { htobuf64(buf, htobe64(big64)); } diff --git a/LeaseSet.cpp b/LeaseSet.cpp index 15c2ded3..1a2c7d39 100644 --- a/LeaseSet.cpp +++ b/LeaseSet.cpp @@ -1,3 +1,4 @@ +#include #include "I2PEndian.h" #include #include "CryptoConst.h" @@ -41,14 +42,15 @@ namespace data // leases for (auto it: tunnels) { - Lease * lease = (Lease *)(m_Buffer + m_BufferLen); - memcpy (lease->tunnelGateway, it->GetNextIdentHash (), 32); - lease->tunnelID = htobe32 (it->GetNextTunnelID ()); + Lease lease; + memcpy (lease.tunnelGateway, it->GetNextIdentHash (), 32); + lease.tunnelID = htobe32 (it->GetNextTunnelID ()); uint64_t ts = it->GetCreationTime () + i2p::tunnel::TUNNEL_EXPIRATION_TIMEOUT - 60; // 1 minute before expiration ts *= 1000; // in milliseconds - lease->endDate = htobe64 (ts); + lease.endDate = htobe64 (ts); + memcpy(m_Buffer + m_BufferLen, &lease, sizeof(Lease)); m_BufferLen += sizeof (Lease); - } + } // signature localDestination->Sign (m_Buffer, m_BufferLen, m_Buffer + m_BufferLen); m_BufferLen += localDestination->GetIdentity ().GetSignatureLen (); @@ -79,7 +81,8 @@ namespace data const uint8_t * leases = m_Buffer + size; for (int i = 0; i < num; i++) { - Lease lease = *(Lease *)leases; + Lease lease; + memcpy (&lease, leases, sizeof(Lease)); lease.tunnelID = be32toh (lease.tunnelID); lease.endDate = be64toh (lease.endDate); m_Leases.push_back (lease); diff --git a/NetDb.cpp b/NetDb.cpp index 9cd7a924..b4d63631 100644 --- a/NetDb.cpp +++ b/NetDb.cpp @@ -1,3 +1,4 @@ +#include #include "I2PEndian.h" #include #include @@ -424,14 +425,15 @@ namespace data { const uint8_t * buf = m->GetPayload (); size_t len = be16toh (m->GetHeader ()->size); - I2NPDatabaseStoreMsg * msg = (I2NPDatabaseStoreMsg *)buf; + I2NPDatabaseStoreMsg msg; + memcpy (&msg, buf, sizeof (I2NPDatabaseStoreMsg)); size_t offset = sizeof (I2NPDatabaseStoreMsg); - if (msg->replyToken) + if (msg.replyToken) offset += 36; - if (msg->type) + if (msg.type) { LogPrint ("LeaseSet"); - AddLeaseSet (msg->key, buf + offset, len - offset, m->from); + AddLeaseSet (msg.key, buf + offset, len - offset, m->from); } else { @@ -449,7 +451,7 @@ namespace data uint8_t uncompressed[2048]; size_t uncomressedSize = decompressor.MaxRetrievable (); decompressor.Get (uncompressed, uncomressedSize); - AddRouterInfo (msg->key, uncompressed, uncomressedSize); + AddRouterInfo (msg.key, uncompressed, uncomressedSize); } i2p::DeleteI2NPMessage (m); } diff --git a/SSUSession.cpp b/SSUSession.cpp index e8e82d9b..41a24470 100644 --- a/SSUSession.cpp +++ b/SSUSession.cpp @@ -121,6 +121,7 @@ namespace transport void SSUSession::ProcessMessage (uint8_t * buf, size_t len, const boost::asio::ip::udp::endpoint& senderEndpoint) { + //TODO: since we are accessing a uint8_t this is unlikely to crash due to alignment but should be improved SSUHeader * header = (SSUHeader *)buf; switch (header->GetPayloadType ()) { @@ -228,6 +229,7 @@ namespace transport size_t signatureLen = m_RemoteIdentity.GetSignatureLen (); size_t paddingSize = signatureLen & 0x0F; // %16 if (paddingSize > 0) signatureLen += (16 - paddingSize); + //TODO: since we are accessing a uint8_t this is unlikely to crash due to alignment but should be improved m_SessionKeyDecryption.SetIV (((SSUHeader *)buf)->iv); m_SessionKeyDecryption.Decrypt (payload, signatureLen, payload); // verify @@ -600,10 +602,11 @@ namespace transport LogPrint (eLogError, "Unexpected SSU packet length ", len); return; } + //TODO: we are using a dirty solution here but should work for now SSUHeader * header = (SSUHeader *)buf; memcpy (header->iv, iv, 16); header->flag = payloadType << 4; // MSB is 0 - header->time = htobe32 (i2p::util::GetSecondsSinceEpoch ()); + htobe32buf (&(header->time), i2p::util::GetSecondsSinceEpoch ()); uint8_t * encrypted = &header->flag; uint16_t encryptedLen = len - (encrypted - buf); i2p::crypto::CBCEncryption encryption; @@ -623,11 +626,12 @@ namespace transport LogPrint (eLogError, "Unexpected SSU packet length ", len); return; } + //TODO: we are using a dirty solution here but should work for now SSUHeader * header = (SSUHeader *)buf; i2p::context.GetRandomNumberGenerator ().GenerateBlock (header->iv, 16); // random iv m_SessionKeyEncryption.SetIV (header->iv); header->flag = payloadType << 4; // MSB is 0 - header->time = htobe32 (i2p::util::GetSecondsSinceEpoch ()); + htobe32buf (&(header->time), i2p::util::GetSecondsSinceEpoch ()); uint8_t * encrypted = &header->flag; uint16_t encryptedLen = len - (encrypted - buf); m_SessionKeyEncryption.Encrypt (encrypted, encryptedLen, encrypted); @@ -644,6 +648,7 @@ namespace transport LogPrint (eLogError, "Unexpected SSU packet length ", len); return; } + //TODO: since we are accessing a uint8_t this is unlikely to crash due to alignment but should be improved SSUHeader * header = (SSUHeader *)buf; uint8_t * encrypted = &header->flag; uint16_t encryptedLen = len - (encrypted - buf); @@ -660,6 +665,7 @@ namespace transport LogPrint (eLogError, "Unexpected SSU packet length ", len); return; } + //TODO: since we are accessing a uint8_t this is unlikely to crash due to alignment but should be improved SSUHeader * header = (SSUHeader *)buf; uint8_t * encrypted = &header->flag; uint16_t encryptedLen = len - (encrypted - buf); @@ -677,6 +683,7 @@ namespace transport LogPrint (eLogError, "Unexpected SSU packet length ", len); return false; } + //TODO: since we are accessing a uint8_t this is unlikely to crash due to alignment but should be improved SSUHeader * header = (SSUHeader *)buf; uint8_t * encrypted = &header->flag; uint16_t encryptedLen = len - (encrypted - buf); diff --git a/Tunnel.cpp b/Tunnel.cpp index a1a8387a..e3b2d88d 100644 --- a/Tunnel.cpp +++ b/Tunnel.cpp @@ -1,3 +1,4 @@ +#include #include "I2PEndian.h" #include #include @@ -41,6 +42,7 @@ namespace tunnel std::random_shuffle (recordIndicies.begin(), recordIndicies.end()); // create real records + //TODO: this is likely to arise alignment issues but I need to see how I fix it I2NPBuildRequestRecordElGamalEncrypted * records = (I2NPBuildRequestRecordElGamalEncrypted *)(msg->GetPayload () + 1); TunnelHopConfig * hop = m_Config->GetFirstHop (); int i = 0; @@ -126,9 +128,10 @@ namespace tunnel hop = m_Config->GetFirstHop (); while (hop) { - I2NPBuildResponseRecord * record = (I2NPBuildResponseRecord *)(msg + 1 + hop->recordIndex*sizeof (I2NPBuildResponseRecord)); - LogPrint ("Ret code=", (int)record->ret); - if (record->ret) + I2NPBuildResponseRecord record; + memcpy (&record, msg + 1 + hop->recordIndex*sizeof (I2NPBuildResponseRecord), sizeof (I2NPBuildResponseRecord)); + LogPrint ("Ret code=", (int)record.ret); + if (record.ret) // if any of participants declined the tunnel is not established established = false; hop = hop->next;