[netdata] skip invalid TLVs in Network Data during iteration/search (#11283)

This commit enhances `NetworkData` modules to skip invalid TLVs during
iteration or search operations. Specifically, `NetworkDataTlv::Find
()` and `NetworkData::Iterate()` are updated. These two methods are
the primary methods used by all other methods for parsing or
searching for TLVs.

Generally, the leader validates TLVs before registration using
`NetworkData::Leader::Validate()`. However, this change improves
robustness by allowing receivers to also handle possible malformed
Network Data.

This commit also updates the `test_network_data` unit test to cover
the new behavior of skipping invalid TLVs.
This commit is contained in:
Abtin Keshavarzian
2025-02-24 08:55:34 -08:00
committed by GitHub
parent bc54d67ed8
commit 522f590fb6
4 changed files with 70 additions and 10 deletions
+8 -5
View File
@@ -150,6 +150,10 @@ Error NetworkData::Iterate(Iterator &aIterator, uint16_t aRloc16, Config &aConfi
Error error = kErrorNotFound;
NetworkDataIterator iterator(aIterator);
bool shouldIterateOverPrefixTlvs;
shouldIterateOverPrefixTlvs = ((aConfig.mOnMeshPrefix != nullptr) || (aConfig.mExternalRoute != nullptr) ||
(aConfig.mLowpanContext != nullptr));
for (const NetworkDataTlv *cur;
cur = iterator.GetTlv(mTlvs), (cur + 1 <= GetTlvsEnd()) && (cur->GetNext() <= GetTlvsEnd());
@@ -160,14 +164,13 @@ Error NetworkData::Iterate(Iterator &aIterator, uint16_t aRloc16, Config &aConfi
switch (cur->GetType())
{
case NetworkDataTlv::kTypePrefix:
if ((aConfig.mOnMeshPrefix != nullptr) || (aConfig.mExternalRoute != nullptr) ||
(aConfig.mLowpanContext != nullptr))
if (shouldIterateOverPrefixTlvs && As<PrefixTlv>(cur)->IsValid())
{
subTlvs = As<PrefixTlv>(cur)->GetSubTlvs();
}
break;
case NetworkDataTlv::kTypeService:
if (aConfig.mService != nullptr)
if ((aConfig.mService != nullptr) && As<ServiceTlv>(cur)->IsValid())
{
subTlvs = As<ServiceTlv>(cur)->GetSubTlvs();
}
@@ -249,7 +252,7 @@ Error NetworkData::Iterate(Iterator &aIterator, uint16_t aRloc16, Config &aConfi
{
const ContextTlv *contextTlv = As<ContextTlv>(subCur);
if (aConfig.mLowpanContext == nullptr)
if ((aConfig.mLowpanContext == nullptr) || !contextTlv->IsValid())
{
continue;
}
@@ -285,7 +288,7 @@ Error NetworkData::Iterate(Iterator &aIterator, uint16_t aRloc16, Config &aConfi
{
const ServerTlv *server = As<ServerTlv>(subCur);
if (!iterator.IsNewEntry())
if (!iterator.IsNewEntry() || !server->IsValid())
{
continue;
}
+29 -2
View File
@@ -39,13 +39,40 @@ namespace NetworkData {
//---------------------------------------------------------------------------------------------------------------------
// NetworkDataTlv
bool NetworkDataTlv::IsTlvValid(const NetworkDataTlv *aTlv)
{
bool isValid = true;
switch (aTlv->GetType())
{
case kTypePrefix:
isValid = As<PrefixTlv>(aTlv)->IsValid();
break;
case kTypeContext:
isValid = As<ContextTlv>(aTlv)->IsValid();
break;
case kTypeService:
isValid = As<ServiceTlv>(aTlv)->IsValid();
break;
case kTypeServer:
isValid = As<ServerTlv>(aTlv)->IsValid();
break;
case kTypeHasRoute:
case kTypeBorderRouter:
case kTypeCommissioningData:
break;
}
return isValid;
}
const NetworkDataTlv *NetworkDataTlv::Find(const NetworkDataTlv *aStart, const NetworkDataTlv *aEnd, Type aType)
{
const NetworkDataTlv *tlv;
for (tlv = aStart; (tlv + 1 <= aEnd) && (tlv->GetNext() <= aEnd); tlv = tlv->GetNext())
{
if (tlv->GetType() == aType)
if ((tlv->GetType() == aType) && IsTlvValid(tlv))
{
ExitNow();
}
@@ -66,7 +93,7 @@ const NetworkDataTlv *NetworkDataTlv::Find(const NetworkDataTlv *aStart,
for (tlv = aStart; (tlv + 1 <= aEnd) && (tlv->GetNext() <= aEnd); tlv = tlv->GetNext())
{
if ((tlv->GetType() == aType) && (tlv->IsStable() == aStable))
if ((tlv->GetType() == aType) && (tlv->IsStable() == aStable) && IsTlvValid(tlv))
{
ExitNow();
}
+10
View File
@@ -363,6 +363,8 @@ private:
static constexpr uint8_t kTypeMask = 0x7f << kTypeOffset;
static constexpr uint8_t kStableMask = 1 << 0;
static bool IsTlvValid(const NetworkDataTlv *aTlv);
uint8_t mType;
uint8_t mLength;
} OT_TOOL_PACKED_END;
@@ -1119,6 +1121,14 @@ public:
mContextLength = aContextLength;
}
/**
* Indicates whether or not the TLV appears to be well-formed.
*
* @retval TRUE If the TLV appears to be well-formed.
* @retval FALSE If the TLV does not appear to be well-formed.
*/
bool IsValid(void) const { return GetLength() >= sizeof(*this) - sizeof(NetworkDataTlv); }
/**
* Indicates whether or not the Compress flag is set.
*
+23 -3
View File
@@ -117,9 +117,25 @@ void TestNetworkDataIterator(void)
VerifyOrQuit(instance != nullptr);
{
// Network Data:
// - An invalid TLV type.
// - An invalid Prefix TLV with prefix length of 129 (and two HasRoute sub-TLVs).
// - An invalid Prefix TLV with short length (length = 1)
// - An invalid Prefix TLV with no prefix.
// - A valid Prefix TLV with two HasRoute sub-TLVs
const uint8_t kNetworkData[] = {
0x08, 0x04, 0x0B, 0x02, 0x00, 0x00, 0x03, 0x14, 0x00, 0x40, 0xFD, 0x00, 0x12, 0x34,
0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0xC8, 0x00, 0x40, 0x01, 0x03, 0x54, 0x00, 0x00,
0xff, 0x03, 0x01, 0x02, 0x03,
0x03, 0x1D, 0x00, 0x81, 0xFD, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xAA, 0xBB,
0xCC, 0xDD, 0xEE, 0xFF, 0x00, 0x00, 0x03, 0xb8, 0x00, 0x40, 0x01, 0x03, 0x14, 0x00, 0x00,
0x03, 0x01, 0x00,
0x03, 0x02, 0x00, 0x40,
0x03, 0x14, 0x00, 0x40, 0xFD, 0x00, 0x12, 0x34, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0xC8, 0x00,
0x40, 0x01, 0x03, 0x54, 0x00, 0x00,
};
otExternalRouteConfig routes[] = {
@@ -168,6 +184,8 @@ void TestNetworkDataIterator(void)
VerifyOrQuit(CompareExternalRouteConfig(rconfig, route));
}
VerifyOrQuit(netData.GetNextExternalRoute(iter, rconfig) == kErrorNotFound);
netData.FindRlocs(kAnyBrOrServer, kAnyRole, rlocs);
VerifyRlocsArray(rlocs, kRlocs);
@@ -667,7 +685,9 @@ void TestNetworkDataDsnSrpServices(void)
};
const uint8_t kNetworkData[] = {
0x0b, 0x08, 0x80, 0x02, 0x5c, 0x02, 0x0d, 0x02, 0x28, 0x00,
0x0b, 0x01, 0x00,
0x0b, 0x0b, 0x80, 0x02, 0x5c, 0x02, 0x0d, 0x01, 0x00, 0x0d, 0x02, 0x28, 0x00,
0x0b, 0x09, 0x81, 0x02, 0x5c, 0xff, 0x0d, 0x03, 0x6c, 0x00, 0x05,