prevent TcpHeader::Deserialize() from asserting; fix a few small bugs in option code

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<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 @@ 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;
 }
 

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;
 

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 ();

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 ();
 }

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);
 

src/internet/model/tcp-socket-base.cc

@@ -903,7 +903,12 @@ TcpSocketBase::DoForwardUp (Ptr<Packet> 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> packet, Ipv4Header header, uint16_t port
     }
 }
 
+// XXX this is duplicate code with the other DoForwardUp()
 void
 TcpSocketBase::DoForwardUp (Ptr<Packet> packet, Ipv6Header header, uint16_t port)
 {
@@ -1001,7 +1007,12 @@ TcpSocketBase::DoForwardUp (Ptr<Packet> 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);