diff --git a/src/internet/model/tcp-header.cc b/src/internet/model/tcp-header.cc index c3dd30005..e9ba31963 100644 --- a/src/internet/model/tcp-header.cc +++ b/src/internet/model/tcp-header.cc @@ -301,7 +301,7 @@ TcpHeader::Print (std::ostream &os) const uint32_t TcpHeader::GetSerializedSize (void) const { - return 4*GetLength (); + return CalculateHeaderLength () * 4; } void @@ -367,7 +367,11 @@ TcpHeader::Deserialize (Buffer::Iterator start) // Deserialize options if they exist m_options.clear (); uint32_t optionLen = (m_length - 5) * 4; - NS_ASSERT_MSG (optionLen <= 40, "Illegal TCP option length"); + if (optionLen > 40) + { + NS_LOG_ERROR ("Illegal TCP option length " << optionLen << "; options discarded"); + return 20; + } while (optionLen) { uint8_t kind = i.PeekU8 (); @@ -383,7 +387,11 @@ TcpHeader::Deserialize (Buffer::Iterator start) NS_LOG_WARN ("Option kind " << static_cast (kind) << " unknown, skipping."); } optionSize = op->Deserialize (i); - NS_ASSERT_MSG (optionSize == op->GetSerializedSize (), "Option did not deserialize correctly"); + if (optionSize != op->GetSerializedSize ()) + { + NS_LOG_ERROR ("Option did not deserialize correctly"); + break; + } if (optionLen >= optionSize) { optionLen -= optionSize; @@ -392,12 +400,14 @@ TcpHeader::Deserialize (Buffer::Iterator start) } else { - NS_ASSERT_MSG (false, "Option exceeds TCP option space"); + NS_LOG_ERROR ("Option exceeds TCP option space; option discarded"); + break; } if (op->GetKind () == TcpOption::END) { while (optionLen) { + // Discard padding bytes without adding to option list i.Next (1); --optionLen; } @@ -406,7 +416,7 @@ TcpHeader::Deserialize (Buffer::Iterator start) if (m_length != CalculateHeaderLength ()) { - NS_LOG_WARN ("Mismatch between calculated length and in-header value"); + NS_LOG_ERROR ("Mismatch between calculated length and in-header value"); } // Do checksum @@ -431,7 +441,11 @@ TcpHeader::CalculateHeaderLength () const { len += (*i)->GetSerializedSize (); } - + // Option list may not include padding; need to pad up to word boundary + if (len % 4) + { + len += 4 - (len % 4); + } return len >> 2; } diff --git a/src/internet/model/tcp-header.h b/src/internet/model/tcp-header.h index 9e89df349..180bdf166 100644 --- a/src/internet/model/tcp-header.h +++ b/src/internet/model/tcp-header.h @@ -275,7 +275,7 @@ private: * Given the standard size of the header, the method checks for options * and calculates the real length (in words). * - * \return header length + * \return header length in 4-byte words */ uint8_t CalculateHeaderLength () const; diff --git a/src/internet/model/tcp-option-ts.cc b/src/internet/model/tcp-option-ts.cc index 6ff06df87..4dad1e8ac 100644 --- a/src/internet/model/tcp-option-ts.cc +++ b/src/internet/model/tcp-option-ts.cc @@ -88,7 +88,11 @@ TcpOptionTS::Deserialize (Buffer::Iterator start) } uint8_t size = i.ReadU8 (); - NS_ASSERT (size == 10); + if (size != 10) + { + NS_LOG_WARN ("Malformed Timestamp option"); + return 0; + } m_timestamp = i.ReadNtohU32 (); m_echo = i.ReadNtohU32 (); return GetSerializedSize (); diff --git a/src/internet/model/tcp-option-winscale.cc b/src/internet/model/tcp-option-winscale.cc index d8beeb08d..3667488c5 100644 --- a/src/internet/model/tcp-option-winscale.cc +++ b/src/internet/model/tcp-option-winscale.cc @@ -86,7 +86,11 @@ TcpOptionWinScale::Deserialize (Buffer::Iterator start) return 0; } uint8_t size = i.ReadU8 (); - NS_ASSERT (size == 3); + if (size != 3) + { + NS_LOG_WARN ("Malformed Window Scale option"); + return 0; + } m_scale = i.ReadU8 (); return GetSerializedSize (); } diff --git a/src/internet/model/tcp-option.cc b/src/internet/model/tcp-option.cc index 673635e86..72711ae24 100644 --- a/src/internet/model/tcp-option.cc +++ b/src/internet/model/tcp-option.cc @@ -171,7 +171,11 @@ TcpOptionUnknown::Deserialize (Buffer::Iterator start) NS_LOG_WARN ("Trying to Deserialize an Unknown Option of Kind " << int (m_kind)); m_size = i.ReadU8 (); - NS_ASSERT_MSG ((m_size >= 2) && (m_size < 40), "Unable to parse an Unknown Option of Kind " << int (m_kind) << " with apparent size " << int (m_size)); + if (m_size < 2 || m_size > 40) + { + NS_LOG_WARN ("Unable to parse an unknown option of kind " << int (m_kind) << " with apparent size " << int (m_size)); + return 0; + } i.Read (m_content, m_size-2); diff --git a/src/internet/model/tcp-socket-base.cc b/src/internet/model/tcp-socket-base.cc index 80180c903..19d44b5bb 100644 --- a/src/internet/model/tcp-socket-base.cc +++ b/src/internet/model/tcp-socket-base.cc @@ -903,7 +903,12 @@ TcpSocketBase::DoForwardUp (Ptr packet, Ipv4Header header, uint16_t port // Peel off TCP header and do validity checking TcpHeader tcpHeader; - packet->RemoveHeader (tcpHeader); + uint32_t bytesRemoved = packet->RemoveHeader (tcpHeader); + if (bytesRemoved == 0 || bytesRemoved > 60) + { + NS_LOG_ERROR ("Bytes removed: " << bytesRemoved << " invalid"); + return; // Discard invalid packet + } ReadOptions (tcpHeader); @@ -988,6 +993,7 @@ TcpSocketBase::DoForwardUp (Ptr packet, Ipv4Header header, uint16_t port } } +// XXX this is duplicate code with the other DoForwardUp() void TcpSocketBase::DoForwardUp (Ptr packet, Ipv6Header header, uint16_t port) { @@ -1001,7 +1007,12 @@ TcpSocketBase::DoForwardUp (Ptr packet, Ipv6Header header, uint16_t port // Peel off TCP header and do validity checking TcpHeader tcpHeader; - packet->RemoveHeader (tcpHeader); + uint32_t bytesRemoved = packet->RemoveHeader (tcpHeader); + if (bytesRemoved == 0 || bytesRemoved > 60) + { + NS_LOG_ERROR ("Bytes removed: " << bytesRemoved << " invalid"); + return; // Discard invalid packet + } ReadOptions (tcpHeader);