diff --git a/src/core/common/tlvs.cpp b/src/core/common/tlvs.cpp index 04756d24a..2848e7f0c 100644 --- a/src/core/common/tlvs.cpp +++ b/src/core/common/tlvs.cpp @@ -56,6 +56,24 @@ const uint8_t *Tlv::GetValue(void) const Error Tlv::AppendTo(Message &aMessage) const { return aMessage.AppendBytes(this, static_cast(GetSize())); } +Error Tlv::ParseAndSkipTlv(const Message &aMessage, uint16_t &aOffset) +{ + Error error; + ParsedInfo info; + + SuccessOrExit(error = info.ParseFrom(aMessage, aOffset)); + + // `ParseFrom()` has already validated that the entire TLV is + // present within `aMessage`. This ensures that `aOffset + mSize` + // is less than `aMessage.GetLength()`, and therefore we cannot + // have an overflow here. + + aOffset += info.mSize; + +exit: + return error; +} + Error Tlv::FindTlv(const Message &aMessage, uint8_t aType, uint16_t aMaxSize, Tlv &aTlv) { uint16_t offset; diff --git a/src/core/common/tlvs.hpp b/src/core/common/tlvs.hpp index 1acc80694..7baf529a3 100644 --- a/src/core/common/tlvs.hpp +++ b/src/core/common/tlvs.hpp @@ -244,6 +244,21 @@ public: //------------------------------------------------------------------------------------------------------------------ // Static methods for reading/finding/appending TLVs in a `Message`. + /** + * Parses a TLV in a message at a given offset, validating that it is fully contained within the message and then + * updating the offset to skip over the entire parsed TLV. + * + * Can be used independent of whether the read TLV (from the message) is an Extended TLV or not. + * + * @param[in] aMessage The message to read from. + * @param[in,out] aOffset The offset to read from. On success, it is updated to point after the parsed TLV. + * + * @retval kErrorNone Successfully parsed a TLV from @p aMessage. @p aOffset is updated. + * @retval kErrorParse The TLV was not well-formed or was not fully contained in @p aMessage. + * + */ + static Error ParseAndSkipTlv(const Message &aMessage, uint16_t &aOffset); + /** * Reads a TLV's value in a message at a given offset expecting a minimum length for the value. * diff --git a/src/core/thread/discover_scanner.cpp b/src/core/thread/discover_scanner.cpp index e264a7be8..03e2ca4eb 100644 --- a/src/core/thread/discover_scanner.cpp +++ b/src/core/thread/discover_scanner.cpp @@ -335,12 +335,21 @@ void DiscoverScanner::HandleDiscoveryResponse(Mle::RxInfo &aRxInfo) const // Process MeshCoP TLVs while (offset < end) { - IgnoreError(aRxInfo.mMessage.Read(offset, meshcopTlv)); + SuccessOrExit(error = aRxInfo.mMessage.Read(offset, meshcopTlv)); + + if (meshcopTlv.IsExtended()) + { + SuccessOrExit(error = Tlv::ParseAndSkipTlv(aRxInfo.mMessage, offset)); + VerifyOrExit(offset <= end, error = kErrorParse); + continue; + } + + VerifyOrExit(meshcopTlv.GetSize() + offset <= aRxInfo.mMessage.GetLength(), error = kErrorParse); switch (meshcopTlv.GetType()) { case MeshCoP::Tlv::kDiscoveryResponse: - IgnoreError(aRxInfo.mMessage.Read(offset, discoveryResponse)); + SuccessOrExit(error = aRxInfo.mMessage.Read(offset, discoveryResponse)); VerifyOrExit(discoveryResponse.IsValid(), error = kErrorParse); result.mVersion = discoveryResponse.GetVersion(); result.mIsNative = discoveryResponse.IsNativeCommissioner(); diff --git a/src/core/thread/link_metrics.cpp b/src/core/thread/link_metrics.cpp index a8936f371..840d28e34 100644 --- a/src/core/thread/link_metrics.cpp +++ b/src/core/thread/link_metrics.cpp @@ -146,7 +146,14 @@ void Initiator::HandleReport(const Message &aMessage, uint16_t aOffset, uint16_t { SuccessOrExit(error = aMessage.Read(offset, tlv)); - VerifyOrExit(offset + sizeof(Tlv) + tlv.GetLength() <= endOffset, error = kErrorParse); + if (tlv.IsExtended()) + { + SuccessOrExit(error = Tlv::ParseAndSkipTlv(aMessage, offset)); + VerifyOrExit(offset <= endOffset, error = kErrorParse); + continue; + } + + VerifyOrExit(tlv.GetSize() + offset <= endOffset, error = kErrorParse); // The report must contain either: // - One or more Report Sub-TLVs (in case of success), or @@ -320,6 +327,15 @@ Error Initiator::HandleManagementResponse(const Message &aMessage, const Ip6::Ad SuccessOrExit(error = aMessage.Read(offset, tlv)); + if (tlv.IsExtended()) + { + SuccessOrExit(error = Tlv::ParseAndSkipTlv(aMessage, offset)); + VerifyOrExit(offset <= endOffset, error = kErrorParse); + continue; + } + + VerifyOrExit(tlv.GetSize() + offset <= endOffset, error = kErrorParse); + switch (tlv.GetType()) { case StatusSubTlv::kType: @@ -440,6 +456,15 @@ Error Subject::AppendReport(Message &aMessage, const Message &aRequestMessage, N { SuccessOrExit(error = aRequestMessage.Read(offset, tlv)); + if (tlv.IsExtended()) + { + SuccessOrExit(error = Tlv::ParseAndSkipTlv(aMessage, offset)); + VerifyOrExit(offset <= endOffset, error = kErrorParse); + continue; + } + + VerifyOrExit(tlv.GetSize() + offset <= endOffset, error = kErrorParse); + switch (tlv.GetType()) { case SubTlv::kQueryId: diff --git a/src/core/thread/mle_router.cpp b/src/core/thread/mle_router.cpp index 542cb370b..e03712345 100644 --- a/src/core/thread/mle_router.cpp +++ b/src/core/thread/mle_router.cpp @@ -2637,12 +2637,21 @@ void MleRouter::HandleDiscoveryRequest(RxInfo &aRxInfo) while (offset < end) { - IgnoreError(aRxInfo.mMessage.Read(offset, meshcopTlv)); + SuccessOrExit(error = aRxInfo.mMessage.Read(offset, meshcopTlv)); + + if (meshcopTlv.IsExtended()) + { + SuccessOrExit(error = Tlv::ParseAndSkipTlv(aRxInfo.mMessage, offset)); + VerifyOrExit(offset <= end, error = kErrorParse); + continue; + } + + VerifyOrExit(meshcopTlv.GetSize() + offset <= aRxInfo.mMessage.GetLength(), error = kErrorParse); switch (meshcopTlv.GetType()) { case MeshCoP::Tlv::kDiscoveryRequest: - IgnoreError(aRxInfo.mMessage.Read(offset, discoveryRequestTlv)); + SuccessOrExit(error = aRxInfo.mMessage.Read(offset, discoveryRequestTlv)); VerifyOrExit(discoveryRequestTlv.IsValid(), error = kErrorParse); break;