From d0ea59c5682e9d3bb0d78015998ed732d9ce7a35 Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Fri, 5 Feb 2016 08:44:09 -0500 Subject: [PATCH 1/6] add base64 buffer encoding bounds checking --- Base.h | 5 +++++ Identity.cpp | 25 +++++++++++++++++-------- SAM.cpp | 11 ++++++++--- 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/Base.h b/Base.h index 0c59ef03..d598542b 100644 --- a/Base.h +++ b/Base.h @@ -17,6 +17,11 @@ namespace data size_t Base32ToByteStream (const char * inBuf, size_t len, uint8_t * outBuf, size_t outLen); size_t ByteStreamToBase32 (const uint8_t * InBuf, size_t len, char * outBuf, size_t outLen); + /** + Compute the size for a buffer to contain encoded base64 given that the size of the input is input_size bytes + */ + size_t Base64EncodingBufferSize(const size_t input_size); + template class Tag { diff --git a/Identity.cpp b/Identity.cpp index ac49b711..648c996d 100644 --- a/Identity.cpp +++ b/Identity.cpp @@ -228,26 +228,35 @@ namespace data } size_t IdentityEx::ToBuffer (uint8_t * buf, size_t len) const - { + { + size_t fullLen = GetFullLen(); + if (fullLen > len) { + // buffer is too small and may overflow somewhere else + return 0; + } memcpy (buf, &m_StandardIdentity, DEFAULT_IDENTITY_SIZE); if (m_ExtendedLen > 0 && m_ExtendedBuffer) memcpy (buf + DEFAULT_IDENTITY_SIZE, m_ExtendedBuffer, m_ExtendedLen); - return GetFullLen (); + return fullLen; } size_t IdentityEx::FromBase64(const std::string& s) { - uint8_t buf[1024]; - auto len = Base64ToByteStream (s.c_str(), s.length(), buf, 1024); + const size_t slen = s.length(); + const size_t bufLen = Base64EncodingBufferSize(slen); + uint8_t buf[bufLen]; + auto len = Base64ToByteStream (s.c_str(), slen, buf, 1024); return FromBuffer (buf, len); } std::string IdentityEx::ToBase64 () const { - uint8_t buf[1024]; - char str[1536]; - size_t l = ToBuffer (buf, 1024); - size_t l1 = i2p::data::ByteStreamToBase64 (buf, l, str, 1536); + const size_t bufLen = GetFullLen(); + const size_t strLen = Base64EncodingBufferSize(bufLen); + uint8_t buf[bufLen]; + char str[strLen]; + size_t l = ToBuffer (buf, bufLen); + size_t l1 = i2p::data::ByteStreamToBase64 (buf, l, str, strLen); str[l1] = 0; return std::string (str); } diff --git a/SAM.cpp b/SAM.cpp index a20de13d..d331a035 100644 --- a/SAM.cpp +++ b/SAM.cpp @@ -631,10 +631,15 @@ namespace client m_SocketType = eSAMSocketTypeStream; if (!m_IsSilent) { - // send remote peer address - uint8_t ident[1024]; - size_t l = stream->GetRemoteIdentity ()->ToBuffer (ident, 1024); + // get remote peer address + auto ident_ptr = stream->GetRemoteIdentity(); + size_t ident_len = ident_ptr->GetFullLen(); + uint8_t* ident = new uint8_t[ident_len]; + + // send remote peer address as base64 + size_t l = ident_ptr->ToBuffer (ident, ident_len); size_t l1 = i2p::data::ByteStreamToBase64 (ident, l, (char *)m_StreamBuffer, SAM_SOCKET_BUFFER_SIZE); + delete[] ident; m_StreamBuffer[l1] = '\n'; HandleI2PReceive (boost::system::error_code (), l1 +1); // we send identity like it has been received from stream } From 21090eaa39aefebc7214b39b2aa19d69dca54e39 Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Fri, 5 Feb 2016 08:46:08 -0500 Subject: [PATCH 2/6] forgot to commit Base.cpp changes --- Base.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Base.cpp b/Base.cpp index c9ba407a..f5144eda 100644 --- a/Base.cpp +++ b/Base.cpp @@ -327,7 +327,14 @@ namespace data LogPrint (eLogError, "Compression error ", err); return 0; } - } + } + + // from https://stackoverflow.com/questions/1533113/calculate-the-size-to-a-base-64-encoded-message + size_t Base64EncodingBufferSize(const size_t input_size) { + const size_t code_size = ((input_size * 4) / 3); + const size_t padding_size = (input_size % 3) ? (3 - (input_size % 3)) : 0; + return code_size + padding_size; + } } } From d4febb4e84fd1c45fa5f7dc95ddeaa566eb893ce Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Fri, 5 Feb 2016 08:52:07 -0500 Subject: [PATCH 3/6] * bounds check on Identity::FromBuffer * properly indet last commits --- Identity.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Identity.cpp b/Identity.cpp index 648c996d..932d215f 100644 --- a/Identity.cpp +++ b/Identity.cpp @@ -22,6 +22,10 @@ namespace data size_t Identity::FromBuffer (const uint8_t * buf, size_t len) { + if ( len < DEFAULT_IDENTITY_SIZE ) { + // buffer too small, don't overflow + return 0; + } memcpy (publicKey, buf, DEFAULT_IDENTITY_SIZE); return DEFAULT_IDENTITY_SIZE; } @@ -242,17 +246,17 @@ namespace data size_t IdentityEx::FromBase64(const std::string& s) { - const size_t slen = s.length(); - const size_t bufLen = Base64EncodingBufferSize(slen); + const size_t slen = s.length(); + const size_t bufLen = Base64EncodingBufferSize(slen); uint8_t buf[bufLen]; - auto len = Base64ToByteStream (s.c_str(), slen, buf, 1024); + const size_t len = Base64ToByteStream (s.c_str(), slen, buf, bufLen); return FromBuffer (buf, len); } std::string IdentityEx::ToBase64 () const { - const size_t bufLen = GetFullLen(); - const size_t strLen = Base64EncodingBufferSize(bufLen); + const size_t bufLen = GetFullLen(); + const size_t strLen = Base64EncodingBufferSize(bufLen); uint8_t buf[bufLen]; char str[strLen]; size_t l = ToBuffer (buf, bufLen); From bf38bd5a1d25766c16cddd6d9ed17ba9b552ca4a Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Fri, 5 Feb 2016 10:40:23 -0500 Subject: [PATCH 4/6] * Fill padding with random in NTCP phase3 * Fill padding with random in NTCPSession::CreateMsgBuffer * Silence unused variable warnings in NTCPSession.cpp --- NTCPSession.cpp | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/NTCPSession.cpp b/NTCPSession.cpp index 2b745059..ed987bb9 100644 --- a/NTCPSession.cpp +++ b/NTCPSession.cpp @@ -134,6 +134,7 @@ namespace transport void NTCPSession::HandlePhase1Sent (const boost::system::error_code& ecode, std::size_t bytes_transferred) { + (void) bytes_transferred; if (ecode) { LogPrint (eLogInfo, "NTCP: couldn't send Phase 1 message: ", ecode.message ()); @@ -150,6 +151,7 @@ namespace transport void NTCPSession::HandlePhase1Received (const boost::system::error_code& ecode, std::size_t bytes_transferred) { + (void) bytes_transferred; if (ecode) { LogPrint (eLogInfo, "NTCP: phase 1 read error: ", ecode.message ()); @@ -205,6 +207,7 @@ namespace transport void NTCPSession::HandlePhase2Sent (const boost::system::error_code& ecode, std::size_t bytes_transferred, uint32_t tsB) { + (void) bytes_transferred; if (ecode) { LogPrint (eLogInfo, "NTCP: Couldn't send Phase 2 message: ", ecode.message ()); @@ -221,6 +224,7 @@ namespace transport void NTCPSession::HandlePhase2Received (const boost::system::error_code& ecode, std::size_t bytes_transferred) { + (void) bytes_transferred; if (ecode) { LogPrint (eLogInfo, "NTCP: Phase 2 read error: ", ecode.message (), ". Wrong ident assumed"); @@ -277,7 +281,8 @@ namespace transport if (paddingSize > 0) { paddingSize = 16 - paddingSize; - // TODO: fill padding with random data + // fill padding with random data + RAND_bytes(buf, paddingSize); buf += paddingSize; len += paddingSize; } @@ -297,6 +302,7 @@ namespace transport void NTCPSession::HandlePhase3Sent (const boost::system::error_code& ecode, std::size_t bytes_transferred, uint32_t tsA) { + (void) bytes_transferred; if (ecode) { LogPrint (eLogInfo, "NTCP: Couldn't send Phase 3 message: ", ecode.message ()); @@ -420,6 +426,7 @@ namespace transport void NTCPSession::HandlePhase4Sent (const boost::system::error_code& ecode, std::size_t bytes_transferred) { + (void) bytes_transferred; if (ecode) { LogPrint (eLogWarning, "NTCP: Couldn't send Phase 4 message: ", ecode.message ()); @@ -645,8 +652,11 @@ namespace transport } int rem = (len + 6) & 0x0F; // %16 int padding = 0; - if (rem > 0) padding = 16 - rem; - // TODO: fill padding + if (rem > 0) { + padding = 16 - rem; + // fill with random padding + RAND_bytes(sendBuffer + len + 2, padding); + } htobe32buf (sendBuffer + len + 2 + padding, adler32 (adler32 (0, Z_NULL, 0), sendBuffer, len + 2+ padding)); int l = len + padding + 6; @@ -667,6 +677,7 @@ namespace transport void NTCPSession::HandleSent (const boost::system::error_code& ecode, std::size_t bytes_transferred, std::vector > msgs) { + (void) msgs; m_IsSending = false; if (ecode) { From babcbcbceacb4e284402d27be43ff8f872f218af Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Fri, 5 Feb 2016 12:32:50 -0500 Subject: [PATCH 5/6] use const size_t instead of size_t --- Identity.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Identity.cpp b/Identity.cpp index 932d215f..e32c3a59 100644 --- a/Identity.cpp +++ b/Identity.cpp @@ -233,7 +233,7 @@ namespace data size_t IdentityEx::ToBuffer (uint8_t * buf, size_t len) const { - size_t fullLen = GetFullLen(); + const size_t fullLen = GetFullLen(); if (fullLen > len) { // buffer is too small and may overflow somewhere else return 0; From 9f1b84d6f21f37c99f68740a95df989243ac6d56 Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Fri, 5 Feb 2016 12:39:17 -0500 Subject: [PATCH 6/6] use const size_t instead of size_t --- SAM.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/SAM.cpp b/SAM.cpp index d331a035..7b493eb3 100644 --- a/SAM.cpp +++ b/SAM.cpp @@ -633,12 +633,12 @@ namespace client { // get remote peer address auto ident_ptr = stream->GetRemoteIdentity(); - size_t ident_len = ident_ptr->GetFullLen(); + const size_t ident_len = ident_ptr->GetFullLen(); uint8_t* ident = new uint8_t[ident_len]; // send remote peer address as base64 - size_t l = ident_ptr->ToBuffer (ident, ident_len); - size_t l1 = i2p::data::ByteStreamToBase64 (ident, l, (char *)m_StreamBuffer, SAM_SOCKET_BUFFER_SIZE); + const size_t l = ident_ptr->ToBuffer (ident, ident_len); + const size_t l1 = i2p::data::ByteStreamToBase64 (ident, l, (char *)m_StreamBuffer, SAM_SOCKET_BUFFER_SIZE); delete[] ident; m_StreamBuffer[l1] = '\n'; HandleI2PReceive (boost::system::error_code (), l1 +1); // we send identity like it has been received from stream