From 3e54d9cf37cb2c7f4aa8d988061887f47e32a04d Mon Sep 17 00:00:00 2001 From: sarveshkumarv3 <86755931+sarveshkumarv3@users.noreply.github.com> Date: Mon, 24 Aug 2026 08:59:34 -0700 Subject: [PATCH] [dataset] add additional validation for dataset range and formatting (#13518) --- include/openthread/dataset.h | 6 +++ include/openthread/instance.h | 2 +- src/core/meshcop/dataset.cpp | 69 +++++++++++++++++++---------- src/core/meshcop/extended_panid.cpp | 18 ++++++++ src/core/meshcop/extended_panid.hpp | 10 +++++ src/core/meshcop/meshcop_tlvs.cpp | 19 +++++++- src/core/net/ip6_address.hpp | 16 +++++++ tests/unit/test_dataset.cpp | 58 ++++++++++++++++++++++++ 8 files changed, 173 insertions(+), 25 deletions(-) diff --git a/include/openthread/dataset.h b/include/openthread/dataset.h index b2b08ad22..534f83fd9 100644 --- a/include/openthread/dataset.h +++ b/include/openthread/dataset.h @@ -578,6 +578,12 @@ otError otNetworkNameFromString(otNetworkName *aNetworkName, const char *aNameSt * * This method also checks whether there are duplicated TLVs or the TLVs are not well-formed in the @p aDatasetTlvs. * + * In addition to the TLV lengths, the values of the following TLVs are validated: Channel and Wake-up Channel (the + * channel page must be supported and the channel within range), Channel Mask (well-formed entries), PAN ID (must not + * be the broadcast PAN ID 0xffff), Extended PAN ID (must not be all-zeros or all-ones), Mesh-Local Prefix (must be a + * locally assigned ULA prefix, i.e., `fd00::/8`), Network Name (1 to 16 bytes, valid UTF-8, no control characters), + * and Security Policy. + * * @param[in] aDatasetTlvs A pointer to dataset TLVs. * @param[in] aActive TRUE for Active Dataset, FALSE for Pending Dataset. * diff --git a/include/openthread/instance.h b/include/openthread/instance.h index f0fc191c0..3d5366f66 100644 --- a/include/openthread/instance.h +++ b/include/openthread/instance.h @@ -52,7 +52,7 @@ extern "C" { * * @note This number versions both OpenThread platform and user APIs. */ -#define OPENTHREAD_API_VERSION (617) +#define OPENTHREAD_API_VERSION (618) /** * @addtogroup api-instance diff --git a/src/core/meshcop/dataset.cpp b/src/core/meshcop/dataset.cpp index 8b1c5206b..67e95a4df 100644 --- a/src/core/meshcop/dataset.cpp +++ b/src/core/meshcop/dataset.cpp @@ -128,6 +128,10 @@ bool Dataset::IsTlvValid(const Tlv &aTlv) bool isValid = true; uint8_t minLength = 0; + // Validate the TLV format, i.e., that the value is long enough for + // the TLV type. TLV types whose `IsValid()` does its own length + // checking are not included here. + switch (aTlv.GetType()) { case Tlv::kActiveTimestamp: @@ -139,41 +143,25 @@ bool Dataset::IsTlvValid(const Tlv &aTlv) case Tlv::kDelayTimer: minLength = sizeof(DelayTimerTlv::UintValueType); break; - case Tlv::kPanId: - minLength = sizeof(PanIdTlv::UintValueType); - break; - case Tlv::kExtendedPanId: - minLength = sizeof(ExtendedPanIdTlv::ValueType); - break; case Tlv::kPskc: minLength = sizeof(PskcTlv::ValueType); break; case Tlv::kNetworkKey: minLength = sizeof(NetworkKeyTlv::ValueType); break; + case Tlv::kPanId: + minLength = sizeof(PanIdTlv::UintValueType); + break; + case Tlv::kExtendedPanId: + minLength = sizeof(ExtendedPanIdTlv::ValueType); + break; case Tlv::kMeshLocalPrefix: minLength = sizeof(MeshLocalPrefixTlv::ValueType); break; case Tlv::kChannel: - VerifyOrExit(aTlv.GetLength() >= sizeof(ChannelTlvValue), isValid = false); - isValid = aTlv.ReadValueAs().IsValid(); - break; case Tlv::kWakeupChannel: - VerifyOrExit(aTlv.GetLength() >= sizeof(ChannelTlvValue), isValid = false); - isValid = aTlv.ReadValueAs().IsValid(); + minLength = sizeof(ChannelTlvValue); break; - case Tlv::kNetworkName: - isValid = As(aTlv).IsValid(); - break; - - case Tlv::kSecurityPolicy: - isValid = As(aTlv).IsValid(); - break; - - case Tlv::kChannelMask: - isValid = As(aTlv).IsValid(); - break; - default: break; } @@ -181,6 +169,41 @@ bool Dataset::IsTlvValid(const Tlv &aTlv) if (minLength > 0) { isValid = (aTlv.GetLength() >= minLength); + VerifyOrExit(isValid); + } + + // Validate the TLV value. + + switch (aTlv.GetType()) + { + case Tlv::kPanId: + // The broadcast PAN ID does not identify a network. + isValid = (aTlv.ReadValueAs() != Mac::kPanIdBroadcast); + break; + case Tlv::kExtendedPanId: + isValid = aTlv.ReadValueAs().IsValid(); + break; + case Tlv::kMeshLocalPrefix: + // The Mesh-Local Prefix is required to be a locally assigned ULA prefix. + isValid = aTlv.ReadValueAs().IsLocallyAssignedUla(); + break; + case Tlv::kChannel: + isValid = aTlv.ReadValueAs().IsValid(); + break; + case Tlv::kWakeupChannel: + isValid = aTlv.ReadValueAs().IsValid(); + break; + case Tlv::kNetworkName: + isValid = As(aTlv).IsValid(); + break; + case Tlv::kSecurityPolicy: + isValid = As(aTlv).IsValid(); + break; + case Tlv::kChannelMask: + isValid = As(aTlv).IsValid(); + break; + default: + break; } exit: diff --git a/src/core/meshcop/extended_panid.cpp b/src/core/meshcop/extended_panid.cpp index 3ba8803c4..22702f1e7 100644 --- a/src/core/meshcop/extended_panid.cpp +++ b/src/core/meshcop/extended_panid.cpp @@ -49,5 +49,23 @@ ExtendedPanId::InfoString ExtendedPanId::ToString(void) const Error ExtendedPanId::GenerateRandom(void) { return Random::Crypto::Fill(*this); } +bool ExtendedPanId::IsValid(void) const +{ + // The all-zeros and all-ones Extended PAN IDs are disallowed by + // the Thread specification. Determine both cases in a single pass + // by OR-ing and AND-ing all bytes together. + + uint8_t oredBytes = 0x00; + uint8_t andedBytes = 0xff; + + for (uint8_t byte : m8) + { + oredBytes |= byte; + andedBytes &= byte; + } + + return (oredBytes != 0x00) && (andedBytes != 0xff); +} + } // namespace MeshCoP } // namespace ot diff --git a/src/core/meshcop/extended_panid.hpp b/src/core/meshcop/extended_panid.hpp index 3bbdd1228..64b533320 100644 --- a/src/core/meshcop/extended_panid.hpp +++ b/src/core/meshcop/extended_panid.hpp @@ -75,6 +75,16 @@ public: */ Error GenerateRandom(void); + /** + * Indicates whether or not the Extended PAN Identifier is valid. + * + * The all-zeros and all-ones values are reserved and disallowed by the Thread specification. + * + * @retval TRUE If the Extended PAN Identifier is valid. + * @retval FALSE If the Extended PAN Identifier is not valid. + */ + bool IsValid(void) const; + } OT_TOOL_PACKED_END; } // namespace MeshCoP diff --git a/src/core/meshcop/meshcop_tlvs.cpp b/src/core/meshcop/meshcop_tlvs.cpp index 267a44e28..73f4a4808 100644 --- a/src/core/meshcop/meshcop_tlvs.cpp +++ b/src/core/meshcop/meshcop_tlvs.cpp @@ -58,7 +58,24 @@ void NetworkNameTlv::SetNetworkName(const NameData &aNameData) SetLength(len); } -bool NetworkNameTlv::IsValid(void) const { return IsValidUtf8String(mNetworkName, GetLength()); } +bool NetworkNameTlv::IsValid(void) const +{ + bool isValid = false; + +#if OPENTHREAD_CONFIG_ALLOW_EMPTY_NETWORK_NAME + if (GetLength() == 0) + { + ExitNow(isValid = true); + } +#endif + + VerifyOrExit(IsValueInRange(GetLength(), 1, NetworkName::kMaxSize)); + VerifyOrExit(IsValidUtf8String(mNetworkName, GetLength())); + isValid = true; + +exit: + return isValid; +} Error SteeringDataTlv::CopyTo(SteeringData &aSteeringData) const { diff --git a/src/core/net/ip6_address.hpp b/src/core/net/ip6_address.hpp index 4c746157b..28899d79c 100644 --- a/src/core/net/ip6_address.hpp +++ b/src/core/net/ip6_address.hpp @@ -93,6 +93,22 @@ public: */ Error InitFrom(const Prefix &aPrefix); + /** + * Indicates whether or not the Network Prefix is a locally assigned Unique Local Address (ULA) prefix, i.e., a + * `fd00::/8` prefix. + * + * RFC 4193 defines a ULA prefix as `fc00::/7` followed by the L bit, which is set to one for a locally assigned + * prefix. Section 3.2 of RFC 4193 defines a Global ID generation process for locally assigned prefixes only, so + * `fd00::/8` is the only form a conformant generator can produce. This is what `GenerateRandomUla()` produces. + * + * Note that this is intentionally stricter than `Prefix::IsUniqueLocal()`, which matches the entire `fc00::/7` + * ULA range and is used to recognize prefixes advertised by other devices. + * + * @retval TRUE If the Network Prefix is a locally assigned ULA prefix. + * @retval FALSE If the Network Prefix is not a locally assigned ULA prefix. + */ + bool IsLocallyAssignedUla(void) const { return m8[0] == 0xfd; } + } OT_TOOL_PACKED_END; /** diff --git a/tests/unit/test_dataset.cpp b/tests/unit/test_dataset.cpp index fe773bca9..9528a024d 100644 --- a/tests/unit/test_dataset.cpp +++ b/tests/unit/test_dataset.cpp @@ -36,6 +36,19 @@ namespace ot { namespace MeshCoP { +template Error SetFromTlvsAndValidate(const uint8_t (&aTlvs)[kSize]) +{ + Dataset dataset; + + // `SetFrom()` takes the length as `uint8_t`, so guard against an array + // that would be silently narrowed when passed to it. + static_assert(kSize <= Dataset::kMaxLength, "aTlvs is too long for a Dataset"); + + SuccessOrQuit(dataset.SetFrom(aTlvs, kSize)); + + return dataset.ValidateTlvs(); +} + void TestDataset(void) { static const uint8_t kTlvBytes[] = { @@ -57,6 +70,29 @@ void TestDataset(void) 0x00, 0x03, 0x00, 0x00, 0x1a, 0x00, 0x03, 0x00, 0x00, 0x1a, }; + // PAN ID TLV - the broadcast PAN ID is not allowed. + static const uint8_t kInvalidPanId[] = {0x01, 0x02, 0xff, 0xff}; + static const uint8_t kValidPanId[] = {0x01, 0x02, 0xff, 0xfe}; + + // Extended PAN ID TLV - all-zeros and all-ones are disallowed. + static const uint8_t kInvalidExtPanIdAllZeros[] = {0x02, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; + static const uint8_t kInvalidExtPanIdAllOnes[] = {0x02, 0x08, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}; + static const uint8_t kValidExtPanId[] = {0x02, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; + + // Mesh-Local Prefix TLV - must be a locally assigned ULA prefix (`fd00::/8`). + static const uint8_t kInvalidMeshLocalPrefix[] = {0x07, 0x08, 0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00}; + static const uint8_t kInvalidMeshLocalPrefixLBit[] = {0x07, 0x08, 0xfc, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; + static const uint8_t kValidMeshLocalPrefix[] = {0x07, 0x08, 0xfd, 0xde, 0xad, 0x00, 0xbe, 0xef, 0x00, 0x00}; + + // Network Name TLV - 1 to 16 bytes of valid UTF-8 without control characters. + // A zero-length name is valid only when `ALLOW_EMPTY_NETWORK_NAME` is enabled. + static const uint8_t kEmptyNetworkName[] = {0x03, 0x00}; + static const uint8_t kInvalidTooLongNetworkName[] = {0x03, 0x11, 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', + 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a'}; + static const uint8_t kInvalidUtf8NetworkName[] = {0x03, 0x03, 'a', 0x00, 'b'}; + static const uint8_t kValidMaxLenNetworkName[] = {0x03, 0x10, 'a', 'a', 'a', 'a', 'a', 'a', 'a', + 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a'}; + static const Tlv::Type kDatasetTlvTypes[] = { Tlv::kChannel, Tlv::kPanId, Tlv::kExtendedPanId, Tlv::kNetworkName, Tlv::kPskc, Tlv::kNetworkKey, Tlv::kMeshLocalPrefix, Tlv::kSecurityPolicy, Tlv::kActiveTimestamp, @@ -173,6 +209,28 @@ void TestDataset(void) SuccessOrQuit(dataset.SetFrom(kDuplicateChannels, sizeof(kDuplicateChannels) / 2)); SuccessOrQuit(dataset.ValidateTlvs()); + // Invalid TLV values + + VerifyOrQuit(SetFromTlvsAndValidate(kInvalidPanId) == kErrorParse); + VerifyOrQuit(SetFromTlvsAndValidate(kInvalidExtPanIdAllZeros) == kErrorParse); + VerifyOrQuit(SetFromTlvsAndValidate(kInvalidExtPanIdAllOnes) == kErrorParse); + VerifyOrQuit(SetFromTlvsAndValidate(kInvalidMeshLocalPrefix) == kErrorParse); + VerifyOrQuit(SetFromTlvsAndValidate(kInvalidMeshLocalPrefixLBit) == kErrorParse); +#if OPENTHREAD_CONFIG_ALLOW_EMPTY_NETWORK_NAME + SuccessOrQuit(SetFromTlvsAndValidate(kEmptyNetworkName)); +#else + VerifyOrQuit(SetFromTlvsAndValidate(kEmptyNetworkName) == kErrorParse); +#endif + VerifyOrQuit(SetFromTlvsAndValidate(kInvalidTooLongNetworkName) == kErrorParse); + VerifyOrQuit(SetFromTlvsAndValidate(kInvalidUtf8NetworkName) == kErrorParse); + + // Valid variants of the same TLVs + + SuccessOrQuit(SetFromTlvsAndValidate(kValidPanId)); + SuccessOrQuit(SetFromTlvsAndValidate(kValidExtPanId)); + SuccessOrQuit(SetFromTlvsAndValidate(kValidMeshLocalPrefix)); + SuccessOrQuit(SetFromTlvsAndValidate(kValidMaxLenNetworkName)); + // Combining/Merging TLVs from two Datasets. SuccessOrQuit(dataset.SetFrom(kTlvBytes, sizeof(kTlvBytes)));