network: Fix NixVector deserialization and Packet::Deserialize size accounting for padded sections

Author: Gabrielcarvfer

src/network/model/nix-vector.cc

@@ -235,9 +235,17 @@ NixVector::Deserialize(const uint32_t* buffer, uint32_t size)
     NS_LOG_FUNCTION(this << buffer << size);
     const uint32_t* p = buffer;
 
-    NS_ASSERT_MSG(size >= sizeof(m_totalBitSize),
-                  "NixVector minimum serialized length is " << sizeof(m_totalBitSize) << " bytes");
-    if (size < sizeof(m_totalBitSize))
+    // Widths of the serialized fields: total number of bits in the nix-vector
+    // (always present), number of used bits, epoch, and each 32-bit word of
+    // neighbor indices (only present when the nix-vector is not empty)
+    constexpr uint32_t totalBitSizeFieldBytes = sizeof(m_totalBitSize);
+    constexpr uint32_t usedFieldBytes = sizeof(m_used);
+    constexpr uint32_t epochFieldBytes = sizeof(m_epoch);
+    constexpr uint32_t nixWordBytes = sizeof(uint32_t);
+
+    NS_ASSERT_MSG(size >= totalBitSizeFieldBytes,
+                  "NixVector minimum serialized length is " << totalBitSizeFieldBytes << " bytes");
+    if (size < totalBitSizeFieldBytes)
     {
         // return zero if an entire nix-vector was
         // not deserialized
@@ -248,22 +256,24 @@ NixVector::Deserialize(const uint32_t* buffer, uint32_t size)
 
     if (m_totalBitSize)
     {
-        m_used = *p++;
-
         // NixVector is packed in 32-bit unsigned ints.
         uint32_t nixVectorLength = m_totalBitSize / 32;
         nixVectorLength += (m_totalBitSize % 32) ? 1 : 0;
 
-        NS_ASSERT_MSG(size >= 16 + nixVectorLength,
-                      "NixVector serialized length should have been " << 16 + nixVectorLength
+        const uint32_t expectedSize = totalBitSizeFieldBytes + usedFieldBytes + epochFieldBytes +
+                                      nixVectorLength * nixWordBytes;
+        NS_ASSERT_MSG(size >= expectedSize,
+                      "NixVector serialized length should have been " << expectedSize
                                                                       << " but buffer is shorter");
-        if (size < 16 + nixVectorLength * 4)
+        if (size < expectedSize)
         {
             // return zero if an entire nix-vector was
             // not deserialized
             return 0;
         }
 
+        m_used = *p++;
+
         // make sure the nix-vector
         // is empty
         m_nixVector.clear();

src/network/model/nix-vector.h

@@ -124,7 +124,7 @@ class NixVector : public SimpleRefCount<NixVector>
      *
      * @param buffer points to buffer for deserialization
      * @param size number of bytes to deserialize
-     * @return number of deerialized bytes on success, 0 on failure
+     * @return number of deserialized bytes on success, 0 on failure
      */
     uint32_t Deserialize(const uint32_t* buffer, uint32_t size);
 

src/network/model/packet.cc

@@ -189,7 +189,8 @@ Packet::Packet(const uint8_t* buffer, uint32_t size, bool magic)
       m_nixVector(nullptr)
 {
     NS_ASSERT(magic);
-    Deserialize(buffer, size);
+    [[maybe_unused]] uint32_t deserialized = Deserialize(buffer, size);
+    NS_ASSERT_MSG(deserialized == size, "Packet deserialization failed");
 }
 
 Packet::Packet(const uint8_t* buffer, uint32_t size)
@@ -595,6 +596,28 @@ Packet::EnableChecking()
     PacketMetadata::EnableChecking();
 }
 
+namespace
+{
+
+/// Bytes occupied by the length prefix word that precedes each serialized section.
+constexpr uint32_t SECTION_LENGTH_FIELD_SIZE = sizeof(uint32_t);
+
+/// Serialized sections are padded so that each one starts on a 4-byte boundary.
+constexpr uint32_t SERIALIZATION_ALIGNMENT = sizeof(uint32_t);
+
+/**
+ * @brief Round a byte count up to the next serialization boundary.
+ * @param bytes the unpadded byte count
+ * @return bytes rounded up to a multiple of SERIALIZATION_ALIGNMENT
+ */
+constexpr uint32_t
+AlignToBoundary(uint32_t bytes)
+{
+    return (bytes + SERIALIZATION_ALIGNMENT - 1) & ~(SERIALIZATION_ALIGNMENT - 1);
+}
+
+} // namespace
+
 uint32_t
 Packet::GetSerializedSize() const
 {
@@ -604,46 +627,46 @@ Packet::GetSerializedSize() const
     {
         // increment total size by the size of the nix-vector
         // ensuring 4-byte boundary
-        size += ((m_nixVector->GetSerializedSize() + 3) & (~3));
+        size += AlignToBoundary(m_nixVector->GetSerializedSize());
 
         // add 4-bytes for entry of total length of nix-vector
-        size += 4;
+        size += SECTION_LENGTH_FIELD_SIZE;
     }
     else
     {
         // if no nix-vector, still have to add 4-bytes
         // to account for the entry of total size for
         // nix-vector in the buffer
-        size += 4;
+        size += SECTION_LENGTH_FIELD_SIZE;
     }
 
     // increment total size by size of packet tag list
     // ensuring 4-byte boundary
-    size += ((m_packetTagList.GetSerializedSize() + 3) & (~3));
+    size += AlignToBoundary(m_packetTagList.GetSerializedSize());
 
     // add 4-bytes for entry of total length of packet tag list
-    size += 4;
+    size += SECTION_LENGTH_FIELD_SIZE;
 
     // increment total size by size of byte tag list
     // ensuring 4-byte boundary
-    size += ((m_byteTagList.GetSerializedSize() + 3) & (~3));
+    size += AlignToBoundary(m_byteTagList.GetSerializedSize());
 
     // add 4-bytes for entry of total length of byte tag list
-    size += 4;
+    size += SECTION_LENGTH_FIELD_SIZE;
 
     // increment total size by size of meta-data
     // ensuring 4-byte boundary
-    size += ((m_metadata.GetSerializedSize() + 3) & (~3));
+    size += AlignToBoundary(m_metadata.GetSerializedSize());
 
     // add 4-bytes for entry of total length of meta-data
-    size += 4;
+    size += SECTION_LENGTH_FIELD_SIZE;
 
     // increment total size by size of buffer
     // ensuring 4-byte boundary
-    size += ((m_buffer.GetSerializedSize() + 3) & (~3));
+    size += AlignToBoundary(m_buffer.GetSerializedSize());
 
     // add 4-bytes for entry of total length of buffer
-    size += 4;
+    size += SECTION_LENGTH_FIELD_SIZE;
 
     return size;
 }
@@ -658,15 +681,14 @@ Packet::Serialize(uint8_t* buffer, uint32_t maxSize) const
     if (m_nixVector)
     {
         uint32_t nixSize = m_nixVector->GetSerializedSize();
-        size += nixSize;
+        size += AlignToBoundary(nixSize) + SECTION_LENGTH_FIELD_SIZE;
         if (size > maxSize)
         {
             return 0;
         }
 
         // put the total length of nix-vector in the buffer
         *p++ = nixSize;
-        size += 4;
 
         // serialize the nix-vector
         uint32_t serialized = m_nixVector->Serialize(p, nixSize);
@@ -677,11 +699,11 @@ Packet::Serialize(uint8_t* buffer, uint32_t maxSize) const
 
         // increment p by nixSize bytes
         // ensuring 4-byte boundary
-        p += ((nixSize + 3) & (~3)) / 4;
+        p += AlignToBoundary(nixSize) / SERIALIZATION_ALIGNMENT;
     }
     else
     {
-        size += 4;
+        size += SECTION_LENGTH_FIELD_SIZE;
         if (size > maxSize)
         {
             return 0;
@@ -693,15 +715,14 @@ Packet::Serialize(uint8_t* buffer, uint32_t maxSize) const
 
     // Serialize byte tag list
     uint32_t byteTagSize = m_byteTagList.GetSerializedSize();
-    size += byteTagSize;
+    size += AlignToBoundary(byteTagSize) + SECTION_LENGTH_FIELD_SIZE;
     if (size > maxSize)
     {
         return 0;
     }
 
     // put the total length of byte tag list in the buffer
     *p++ = byteTagSize;
-    size += 4;
 
     // serialize the byte tag list
     uint32_t serialized = m_byteTagList.Serialize(p, byteTagSize);
@@ -712,19 +733,18 @@ Packet::Serialize(uint8_t* buffer, uint32_t maxSize) const
 
     // increment p by byteTagSize bytes
     // ensuring 4-byte boundary
-    p += ((byteTagSize + 3) & (~3)) / 4;
+    p += AlignToBoundary(byteTagSize) / SERIALIZATION_ALIGNMENT;
 
     // Serialize packet tag list
     uint32_t packetTagSize = m_packetTagList.GetSerializedSize();
-    size += packetTagSize;
+    size += AlignToBoundary(packetTagSize) + SECTION_LENGTH_FIELD_SIZE;
     if (size > maxSize)
     {
         return 0;
     }
 
     // put the total length of packet tag list in the buffer
     *p++ = packetTagSize;
-    size += 4;
 
     // serialize the packet tag list
     serialized = m_packetTagList.Serialize(p, packetTagSize);
@@ -735,19 +755,18 @@ Packet::Serialize(uint8_t* buffer, uint32_t maxSize) const
 
     // increment p by packetTagSize bytes
     // ensuring 4-byte boundary
-    p += ((packetTagSize + 3) & (~3)) / 4;
+    p += AlignToBoundary(packetTagSize) / SERIALIZATION_ALIGNMENT;
 
     // Serialize Metadata
     uint32_t metaSize = m_metadata.GetSerializedSize();
-    size += metaSize;
+    size += AlignToBoundary(metaSize) + SECTION_LENGTH_FIELD_SIZE;
     if (size > maxSize)
     {
         return 0;
     }
 
     // put the total length of metadata in the buffer
     *p++ = metaSize;
-    size += 4;
 
     // serialize the metadata
     serialized = m_metadata.Serialize(reinterpret_cast<uint8_t*>(p), metaSize);
@@ -758,11 +777,11 @@ Packet::Serialize(uint8_t* buffer, uint32_t maxSize) const
 
     // increment p by metaSize bytes
     // ensuring 4-byte boundary
-    p += ((metaSize + 3) & (~3)) / 4;
+    p += AlignToBoundary(metaSize) / SERIALIZATION_ALIGNMENT;
 
     // Serialize the packet contents
     uint32_t bufSize = m_buffer.GetSerializedSize();
-    size += bufSize;
+    size += AlignToBoundary(bufSize) + SECTION_LENGTH_FIELD_SIZE;
     if (size > maxSize)
     {
         return 0;
@@ -777,7 +796,7 @@ Packet::Serialize(uint8_t* buffer, uint32_t maxSize) const
     {
         return 0;
     }
-    return size + bufSize + 4;
+    return size;
 }
 
 uint32_t
@@ -794,7 +813,7 @@ Packet::Deserialize(const uint8_t* buffer, uint32_t size)
 
     // if sizeCheck less than nixSize, the buffer
     // will be overrun, assert
-    NS_ASSERT(sizeCheck >= nixSize);
+    NS_ASSERT(sizeCheck >= AlignToBoundary(nixSize) + SECTION_LENGTH_FIELD_SIZE);
 
     if (nixSize > 0)
     {
@@ -809,16 +828,16 @@ Packet::Deserialize(const uint8_t* buffer, uint32_t size)
         m_nixVector = nix;
         // increment p by nixSize ensuring
         // 4-byte boundary
-        p += (((nixSize + 3) & (~3)) / 4);
+        p += AlignToBoundary(nixSize) / SERIALIZATION_ALIGNMENT;
     }
-    sizeCheck -= nixSize + 4;
+    sizeCheck -= AlignToBoundary(nixSize) + SECTION_LENGTH_FIELD_SIZE;
 
     // read byte tags
     uint32_t byteTagSize = *p++;
 
     // if sizeCheck less than byteTagSize, the buffer
     // will be overrun, assert
-    NS_ASSERT(sizeCheck >= byteTagSize);
+    NS_ASSERT(sizeCheck >= AlignToBoundary(byteTagSize) + SECTION_LENGTH_FIELD_SIZE);
 
     uint32_t byteTagDeserialized = m_byteTagList.Deserialize(p, byteTagSize);
     if (!byteTagDeserialized)
@@ -828,15 +847,15 @@ Packet::Deserialize(const uint8_t* buffer, uint32_t size)
     }
     // increment p by byteTagSize ensuring
     // 4-byte boundary
-    p += (((byteTagSize + 3) & (~3)) / 4);
-    sizeCheck -= byteTagSize + 4;
+    p += AlignToBoundary(byteTagSize) / SERIALIZATION_ALIGNMENT;
+    sizeCheck -= AlignToBoundary(byteTagSize) + SECTION_LENGTH_FIELD_SIZE;
 
     // read packet tags
     uint32_t packetTagSize = *p++;
 
     // if sizeCheck less than packetTagSize, the buffer
     // will be overrun, assert
-    NS_ASSERT(sizeCheck >= packetTagSize);
+    NS_ASSERT(sizeCheck >= AlignToBoundary(packetTagSize) + SECTION_LENGTH_FIELD_SIZE);
 
     uint32_t packetTagDeserialized = m_packetTagList.Deserialize(p, packetTagSize);
     if (!packetTagDeserialized)
@@ -846,15 +865,15 @@ Packet::Deserialize(const uint8_t* buffer, uint32_t size)
     }
     // increment p by packetTagSize ensuring
     // 4-byte boundary
-    p += (((packetTagSize + 3) & (~3)) / 4);
-    sizeCheck -= packetTagSize + 4;
+    p += AlignToBoundary(packetTagSize) / SERIALIZATION_ALIGNMENT;
+    sizeCheck -= AlignToBoundary(packetTagSize) + SECTION_LENGTH_FIELD_SIZE;
 
     // read metadata
     uint32_t metaSize = *p++;
 
     // if sizeCheck less than metaSize, the buffer
     // will be overrun, assert
-    NS_ASSERT(sizeCheck >= metaSize);
+    NS_ASSERT(sizeCheck >= AlignToBoundary(metaSize) + SECTION_LENGTH_FIELD_SIZE);
 
     uint32_t metadataDeserialized =
         m_metadata.Deserialize(reinterpret_cast<const uint8_t*>(p), metaSize);
@@ -866,15 +885,15 @@ Packet::Deserialize(const uint8_t* buffer, uint32_t size)
     }
     // increment p by metaSize ensuring
     // 4-byte boundary
-    p += (((metaSize + 3) & (~3)) / 4);
-    sizeCheck -= metaSize + 4;
+    p += AlignToBoundary(metaSize) / SERIALIZATION_ALIGNMENT;
+    sizeCheck -= AlignToBoundary(metaSize) + SECTION_LENGTH_FIELD_SIZE;
 
     // read buffer contents
     uint32_t bufSize = *p++;
 
     // if sizeCheck less than bufSize, the buffer
     // will be overrun, assert
-    NS_ASSERT(sizeCheck >= bufSize);
+    NS_ASSERT(sizeCheck >= AlignToBoundary(bufSize) + SECTION_LENGTH_FIELD_SIZE);
 
     uint32_t bufferDeserialized =
         m_buffer.Deserialize(reinterpret_cast<const uint8_t*>(p), bufSize);
@@ -884,7 +903,7 @@ Packet::Deserialize(const uint8_t* buffer, uint32_t size)
         // completely
         return 0;
     }
-    sizeCheck -= bufSize + 4;
+    sizeCheck -= AlignToBoundary(bufSize) + SECTION_LENGTH_FIELD_SIZE;
 
     // return zero if did not deserialize the
     // number of expected bytes