prevent TcpHeader::Deserialize() from asserting; fix a few small bugs in option code
--- a/src/internet/model/tcp-header.cc Fri Sep 12 18:19:47 2014 -0700
+++ b/src/internet/model/tcp-header.cc Fri Sep 12 18:29:47 2014 -0700
@@ -301,7 +301,7 @@
uint32_t
TcpHeader::GetSerializedSize (void) const
{
- return 4*GetLength ();
+ return CalculateHeaderLength () * 4;
}
void
@@ -367,7 +367,11 @@
// 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 @@
NS_LOG_WARN ("Option kind " << static_cast<int> (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 @@
}
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 @@
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 @@
{
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;
}
--- a/src/internet/model/tcp-header.h Fri Sep 12 18:19:47 2014 -0700
+++ b/src/internet/model/tcp-header.h Fri Sep 12 18:29:47 2014 -0700
@@ -275,7 +275,7 @@
* 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;
--- a/src/internet/model/tcp-option-ts.cc Fri Sep 12 18:19:47 2014 -0700
+++ b/src/internet/model/tcp-option-ts.cc Fri Sep 12 18:29:47 2014 -0700
@@ -88,7 +88,11 @@
}
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 ();
--- a/src/internet/model/tcp-option-winscale.cc Fri Sep 12 18:19:47 2014 -0700
+++ b/src/internet/model/tcp-option-winscale.cc Fri Sep 12 18:29:47 2014 -0700
@@ -86,7 +86,11 @@
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 ();
}
--- a/src/internet/model/tcp-option.cc Fri Sep 12 18:19:47 2014 -0700
+++ b/src/internet/model/tcp-option.cc Fri Sep 12 18:29:47 2014 -0700
@@ -171,7 +171,11 @@
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);
--- a/src/internet/model/tcp-socket-base.cc Fri Sep 12 18:19:47 2014 -0700
+++ b/src/internet/model/tcp-socket-base.cc Fri Sep 12 18:29:47 2014 -0700
@@ -903,7 +903,12 @@
// 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 @@
}
}
+// XXX this is duplicate code with the other DoForwardUp()
void
TcpSocketBase::DoForwardUp (Ptr<Packet> packet, Ipv6Header header, uint16_t port)
{
@@ -1001,7 +1007,12 @@
// 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);