diff --git a/src/core/meshcop/dataset.cpp b/src/core/meshcop/dataset.cpp index 047808594..4588f852a 100644 --- a/src/core/meshcop/dataset.cpp +++ b/src/core/meshcop/dataset.cpp @@ -436,10 +436,12 @@ Error Dataset::SetTlv(const Tlv &aTlv) return SetTlv(aTlv.GetType(), aTlv.GetValue(), aTlv.GetLength()); } -Error Dataset::ReadFromMessage(const Message &aMessage, uint16_t aOffset, uint8_t aLength) +Error Dataset::ReadFromMessage(const Message &aMessage, uint16_t aOffset, uint16_t aLength) { Error error = kErrorParse; + VerifyOrExit(aLength <= kMaxSize); + SuccessOrExit(aMessage.Read(aOffset, mTlvs, aLength)); mLength = aLength; diff --git a/src/core/meshcop/dataset.hpp b/src/core/meshcop/dataset.hpp index 380951373..d45ea0278 100644 --- a/src/core/meshcop/dataset.hpp +++ b/src/core/meshcop/dataset.hpp @@ -795,7 +795,7 @@ public: * @retval kErrorParse Could not read or parse the dataset from @p aMessage. * */ - Error ReadFromMessage(const Message &aMessage, uint16_t aOffset, uint8_t aLength); + Error ReadFromMessage(const Message &aMessage, uint16_t aOffset, uint16_t aLength); /** * This method sets the Dataset using an existing Dataset. diff --git a/src/core/meshcop/dataset_manager.cpp b/src/core/meshcop/dataset_manager.cpp index cc4b3bdd7..c48023ba4 100644 --- a/src/core/meshcop/dataset_manager.cpp +++ b/src/core/meshcop/dataset_manager.cpp @@ -668,7 +668,7 @@ exit: Error ActiveDatasetManager::Save(const Timestamp &aTimestamp, const Message & aMessage, uint16_t aOffset, - uint8_t aLength) + uint16_t aLength) { Error error = kErrorNone; Dataset dataset; @@ -749,7 +749,7 @@ exit: Error PendingDatasetManager::Save(const Timestamp &aTimestamp, const Message & aMessage, uint16_t aOffset, - uint8_t aLength) + uint16_t aLength) { Error error = kErrorNone; Dataset dataset; diff --git a/src/core/meshcop/dataset_manager.hpp b/src/core/meshcop/dataset_manager.hpp index 17cef4f82..40bb59dbc 100644 --- a/src/core/meshcop/dataset_manager.hpp +++ b/src/core/meshcop/dataset_manager.hpp @@ -428,7 +428,7 @@ public: * @retval kErrorParse Could not parse the Dataset from @p aMessage. * */ - Error Save(const Timestamp &aTimestamp, const Message &aMessage, uint16_t aOffset, uint8_t aLength); + Error Save(const Timestamp &aTimestamp, const Message &aMessage, uint16_t aOffset, uint16_t aLength); /** * This method sets the Operational Dataset in non-volatile memory. @@ -562,7 +562,7 @@ public: * @param[in] aLength The length of the Operational Dataset. * */ - Error Save(const Timestamp &aTimestamp, const Message &aMessage, uint16_t aOffset, uint8_t aLength); + Error Save(const Timestamp &aTimestamp, const Message &aMessage, uint16_t aOffset, uint16_t aLength); /** * This method saves the Operational Dataset in non-volatile memory. diff --git a/src/core/thread/discover_scanner.cpp b/src/core/thread/discover_scanner.cpp index 8dfc1380a..89214cbe1 100644 --- a/src/core/thread/discover_scanner.cpp +++ b/src/core/thread/discover_scanner.cpp @@ -292,12 +292,12 @@ void DiscoverScanner::HandleDiscoveryResponse(Mle::RxInfo &aRxInfo) const { Error error = kErrorNone; const ThreadLinkInfo * linkInfo = aRxInfo.mMessageInfo.GetThreadLinkInfo(); - Tlv tlv; MeshCoP::Tlv meshcopTlv; MeshCoP::DiscoveryResponseTlv discoveryResponse; MeshCoP::NetworkNameTlv networkName; ScanResult result; uint16_t offset; + uint16_t length; uint16_t end; bool didCheckSteeringData = false; @@ -306,11 +306,8 @@ void DiscoverScanner::HandleDiscoveryResponse(Mle::RxInfo &aRxInfo) const VerifyOrExit(mState == kStateScanning, error = kErrorDrop); // Find MLE Discovery TLV - VerifyOrExit(Tlv::FindTlvOffset(aRxInfo.mMessage, Tlv::kDiscovery, offset) == kErrorNone, error = kErrorParse); - IgnoreError(aRxInfo.mMessage.Read(offset, tlv)); - - offset += sizeof(tlv); - end = offset + tlv.GetLength(); + SuccessOrExit(error = Tlv::FindTlvValueOffset(aRxInfo.mMessage, Tlv::kDiscovery, offset, length)); + end = offset + length; memset(&result, 0, sizeof(result)); result.mDiscover = true; diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp index 83387165a..439e95f32 100644 --- a/src/core/thread/mle.cpp +++ b/src/core/thread/mle.cpp @@ -2838,13 +2838,15 @@ Error Mle::HandleLeaderData(RxInfo &aRxInfo) MeshCoP::Timestamp activeTimestamp; MeshCoP::Timestamp pendingTimestamp; const MeshCoP::Timestamp *timestamp; - bool hasActiveTimestamp = false; - bool hasPendingTimestamp = false; - uint16_t networkDataOffset = 0; + bool hasActiveTimestamp = false; + bool hasPendingTimestamp = false; + uint16_t networkDataOffset; + uint16_t networkDataLength; uint16_t activeDatasetOffset = 0; + uint16_t activeDatasetLength = 0; uint16_t pendingDatasetOffset = 0; + uint16_t pendingDatasetLength = 0; bool dataRequest = false; - Tlv tlv; // Leader Data SuccessOrExit(error = aRxInfo.mMessage.ReadLeaderDataTlv(leaderData)); @@ -2878,7 +2880,8 @@ Error Mle::HandleLeaderData(RxInfo &aRxInfo) // if received timestamp does not match the local value and message does not contain the dataset, // send MLE Data Request if (!IsLeader() && (MeshCoP::Timestamp::Compare(&activeTimestamp, timestamp) != 0) && - (Tlv::FindTlvOffset(aRxInfo.mMessage, Tlv::kActiveDataset, activeDatasetOffset) != kErrorNone)) + (Tlv::FindTlvValueOffset(aRxInfo.mMessage, Tlv::kActiveDataset, activeDatasetOffset, activeDatasetLength) != + kErrorNone)) { ExitNow(dataRequest = true); } @@ -2903,7 +2906,8 @@ Error Mle::HandleLeaderData(RxInfo &aRxInfo) // if received timestamp does not match the local value and message does not contain the dataset, // send MLE Data Request if (!IsLeader() && (MeshCoP::Timestamp::Compare(&pendingTimestamp, timestamp) != 0) && - (Tlv::FindTlvOffset(aRxInfo.mMessage, Tlv::kPendingDataset, pendingDatasetOffset) != kErrorNone)) + (Tlv::FindTlvValueOffset(aRxInfo.mMessage, Tlv::kPendingDataset, pendingDatasetOffset, + pendingDatasetLength) != kErrorNone)) { ExitNow(dataRequest = true); } @@ -2917,11 +2921,12 @@ Error Mle::HandleLeaderData(RxInfo &aRxInfo) ExitNow(error = kErrorParse); } - if (Tlv::FindTlvOffset(aRxInfo.mMessage, Tlv::kNetworkData, networkDataOffset) == kErrorNone) + if (Tlv::FindTlvValueOffset(aRxInfo.mMessage, Tlv::kNetworkData, networkDataOffset, networkDataLength) == + kErrorNone) { - error = Get().SetNetworkData(leaderData.GetDataVersion(NetworkData::kFullSet), - leaderData.GetDataVersion(NetworkData::kStableSubset), - GetNetworkDataType(), aRxInfo.mMessage, networkDataOffset); + error = Get().SetNetworkData( + leaderData.GetDataVersion(NetworkData::kFullSet), leaderData.GetDataVersion(NetworkData::kStableSubset), + GetNetworkDataType(), aRxInfo.mMessage, networkDataOffset, networkDataLength); SuccessOrExit(error); } else @@ -2942,9 +2947,8 @@ Error Mle::HandleLeaderData(RxInfo &aRxInfo) { if (activeDatasetOffset > 0) { - IgnoreError(aRxInfo.mMessage.Read(activeDatasetOffset, tlv)); - IgnoreError(Get().Save( - activeTimestamp, aRxInfo.mMessage, activeDatasetOffset + sizeof(tlv), tlv.GetLength())); + IgnoreError(Get().Save(activeTimestamp, aRxInfo.mMessage, + activeDatasetOffset, activeDatasetLength)); } } @@ -2953,9 +2957,8 @@ Error Mle::HandleLeaderData(RxInfo &aRxInfo) { if (pendingDatasetOffset > 0) { - IgnoreError(aRxInfo.mMessage.Read(pendingDatasetOffset, tlv)); - IgnoreError(Get().Save( - pendingTimestamp, aRxInfo.mMessage, pendingDatasetOffset + sizeof(tlv), tlv.GetLength())); + IgnoreError(Get().Save(pendingTimestamp, aRxInfo.mMessage, + pendingDatasetOffset, pendingDatasetLength)); } } } @@ -3285,9 +3288,10 @@ void Mle::HandleChildIdResponse(RxInfo &aRxInfo) uint16_t sourceAddress; uint16_t shortAddress; MeshCoP::Timestamp timestamp; - Tlv tlv; uint16_t networkDataOffset; + uint16_t networkDataLength; uint16_t offset; + uint16_t length; // Source Address SuccessOrExit(error = Tlv::Find(aRxInfo.mMessage, sourceAddress)); @@ -3306,18 +3310,18 @@ void Mle::HandleChildIdResponse(RxInfo &aRxInfo) SuccessOrExit(error = aRxInfo.mMessage.ReadLeaderDataTlv(leaderData)); // Network Data - SuccessOrExit(error = Tlv::FindTlvOffset(aRxInfo.mMessage, Tlv::kNetworkData, networkDataOffset)); + SuccessOrExit( + error = Tlv::FindTlvValueOffset(aRxInfo.mMessage, Tlv::kNetworkData, networkDataOffset, networkDataLength)); // Active Timestamp switch (Tlv::Find(aRxInfo.mMessage, timestamp)) { case kErrorNone: // Active Dataset - if (Tlv::FindTlvOffset(aRxInfo.mMessage, Tlv::kActiveDataset, offset) == kErrorNone) + if (Tlv::FindTlvValueOffset(aRxInfo.mMessage, Tlv::kActiveDataset, offset, length) == kErrorNone) { - IgnoreError(aRxInfo.mMessage.Read(offset, tlv)); - SuccessOrExit(error = Get().Save(timestamp, aRxInfo.mMessage, - offset + sizeof(tlv), tlv.GetLength())); + SuccessOrExit(error = + Get().Save(timestamp, aRxInfo.mMessage, offset, length)); } break; @@ -3339,11 +3343,9 @@ void Mle::HandleChildIdResponse(RxInfo &aRxInfo) { case kErrorNone: // Pending Dataset - if (Tlv::FindTlvOffset(aRxInfo.mMessage, Tlv::kPendingDataset, offset) == kErrorNone) + if (Tlv::FindTlvValueOffset(aRxInfo.mMessage, Tlv::kPendingDataset, offset, length) == kErrorNone) { - IgnoreError(aRxInfo.mMessage.Read(offset, tlv)); - IgnoreError(Get().Save(timestamp, aRxInfo.mMessage, offset + sizeof(tlv), - tlv.GetLength())); + IgnoreError(Get().Save(timestamp, aRxInfo.mMessage, offset, length)); } break; @@ -3392,9 +3394,9 @@ void Mle::HandleChildIdResponse(RxInfo &aRxInfo) mParent.SetRloc16(sourceAddress); - IgnoreError(Get().SetNetworkData(leaderData.GetDataVersion(NetworkData::kFullSet), - leaderData.GetDataVersion(NetworkData::kStableSubset), - GetNetworkDataType(), aRxInfo.mMessage, networkDataOffset)); + IgnoreError(Get().SetNetworkData( + leaderData.GetDataVersion(NetworkData::kFullSet), leaderData.GetDataVersion(NetworkData::kStableSubset), + GetNetworkDataType(), aRxInfo.mMessage, networkDataOffset, networkDataLength)); SetStateChild(shortAddress); diff --git a/src/core/thread/mle_router.cpp b/src/core/thread/mle_router.cpp index ef2cc90f0..12237e163 100644 --- a/src/core/thread/mle_router.cpp +++ b/src/core/thread/mle_router.cpp @@ -2104,17 +2104,15 @@ exit: } #endif -Error MleRouter::UpdateChildAddresses(const Message &aMessage, uint16_t aOffset, Child &aChild) +Error MleRouter::UpdateChildAddresses(const Message &aMessage, uint16_t aOffset, uint16_t aLength, Child &aChild) { Error error = kErrorNone; AddressRegistrationEntry entry; Ip6::Address address; Lowpan::Context context; - Tlv tlv; uint8_t registeredCount = 0; uint8_t storedCount = 0; - uint16_t offset = 0; - uint16_t end = 0; + uint16_t end = aOffset + aLength; #if OPENTHREAD_FTD && OPENTHREAD_CONFIG_TMF_PROXY_DUA_ENABLE Ip6::Address oldDua; const Ip6::Address *oldDuaPtr = nullptr; @@ -2126,12 +2124,6 @@ Error MleRouter::UpdateChildAddresses(const Message &aMessage, uint16_t aOffset, uint16_t oldMlrRegisteredAddressNum = 0; #endif - SuccessOrExit(error = aMessage.Read(aOffset, tlv)); - VerifyOrExit(tlv.GetLength() <= (aMessage.GetLength() - aOffset - sizeof(tlv)), error = kErrorParse); - - offset = aOffset + sizeof(tlv); - end = offset + tlv.GetLength(); - #if OPENTHREAD_FTD && OPENTHREAD_CONFIG_TMF_PROXY_DUA_ENABLE if ((oldDuaPtr = aChild.GetDomainUnicastAddress()) != nullptr) { @@ -2158,18 +2150,18 @@ Error MleRouter::UpdateChildAddresses(const Message &aMessage, uint16_t aOffset, aChild.ClearIp6Addresses(); - while (offset < end) + while (aOffset < end) { uint8_t len; // read out the control field - SuccessOrExit(error = aMessage.Read(offset, &entry, sizeof(uint8_t))); + SuccessOrExit(error = aMessage.Read(aOffset, &entry, sizeof(uint8_t))); len = entry.GetLength(); - SuccessOrExit(error = aMessage.Read(offset, &entry, len)); + SuccessOrExit(error = aMessage.Read(aOffset, &entry, len)); - offset += len; + aOffset += len; registeredCount++; if (entry.IsCompressed()) @@ -2317,7 +2309,6 @@ void MleRouter::HandleChildIdRequest(RxInfo &aRxInfo) Child * child; Router * router; uint8_t numTlvs; - uint16_t addressRegistrationOffset = 0; Log(kMessageReceive, kTypeChildIdRequest, aRxInfo.mMessageInfo.GetPeerAddr()); @@ -2403,9 +2394,11 @@ void MleRouter::HandleChildIdRequest(RxInfo &aRxInfo) if (!mode.IsFullThreadDevice()) { - SuccessOrExit(error = - Tlv::FindTlvOffset(aRxInfo.mMessage, Tlv::kAddressRegistration, addressRegistrationOffset)); - SuccessOrExit(error = UpdateChildAddresses(aRxInfo.mMessage, addressRegistrationOffset, *child)); + uint16_t offset; + uint16_t length; + + SuccessOrExit(error = Tlv::FindTlvValueOffset(aRxInfo.mMessage, Tlv::kAddressRegistration, offset, length)); + SuccessOrExit(error = UpdateChildAddresses(aRxInfo.mMessage, offset, length, *child)); } // Remove from router table @@ -2492,8 +2485,9 @@ void MleRouter::HandleChildUpdateRequest(RxInfo &aRxInfo) DeviceMode oldMode; TlvList requestedTlvList; TlvList tlvList; - uint16_t addressRegistrationOffset = 0; - bool childDidChange = false; + uint16_t addrOffset; + uint16_t addrLength; + bool childDidChange = false; Log(kMessageReceive, kTypeChildUpdateRequestOfChild, aRxInfo.mMessageInfo.GetPeerAddr()); @@ -2555,9 +2549,9 @@ void MleRouter::HandleChildUpdateRequest(RxInfo &aRxInfo) } // IPv6 Address TLV - if (Tlv::FindTlvOffset(aRxInfo.mMessage, Tlv::kAddressRegistration, addressRegistrationOffset) == kErrorNone) + if (Tlv::FindTlvValueOffset(aRxInfo.mMessage, Tlv::kAddressRegistration, addrOffset, addrLength) == kErrorNone) { - SuccessOrExit(error = UpdateChildAddresses(aRxInfo.mMessage, addressRegistrationOffset, *child)); + SuccessOrExit(error = UpdateChildAddresses(aRxInfo.mMessage, addrOffset, addrLength, *child)); tlvList.Add(Tlv::kAddressRegistration); } @@ -2695,7 +2689,8 @@ void MleRouter::HandleChildUpdateResponse(RxInfo &aRxInfo) uint32_t mleFrameCounter; LeaderData leaderData; Child * child; - uint16_t addressRegistrationOffset = 0; + uint16_t addrOffset; + uint16_t addrLength; if ((aRxInfo.mNeighbor == nullptr) || IsActiveRouter(aRxInfo.mNeighbor->GetRloc16()) || !Get().Contains(*aRxInfo.mNeighbor)) @@ -2792,9 +2787,9 @@ void MleRouter::HandleChildUpdateResponse(RxInfo &aRxInfo) } // IPv6 Address - if (Tlv::FindTlvOffset(aRxInfo.mMessage, Tlv::kAddressRegistration, addressRegistrationOffset) == kErrorNone) + if (Tlv::FindTlvValueOffset(aRxInfo.mMessage, Tlv::kAddressRegistration, addrOffset, addrLength) == kErrorNone) { - SuccessOrExit(error = UpdateChildAddresses(aRxInfo.mMessage, addressRegistrationOffset, *child)); + SuccessOrExit(error = UpdateChildAddresses(aRxInfo.mMessage, addrOffset, addrLength, *child)); } // Leader Data @@ -2953,11 +2948,11 @@ void MleRouter::SetSteeringData(const Mac::ExtAddress *aExtAddress) void MleRouter::HandleDiscoveryRequest(RxInfo &aRxInfo) { Error error = kErrorNone; - Tlv tlv; MeshCoP::Tlv meshcopTlv; MeshCoP::DiscoveryRequestTlv discoveryRequest; MeshCoP::ExtendedPanId extPanId; uint16_t offset; + uint16_t length; uint16_t end; Log(kMessageReceive, kTypeDiscoveryRequest, aRxInfo.mMessageInfo.GetPeerAddr()); @@ -2967,12 +2962,8 @@ void MleRouter::HandleDiscoveryRequest(RxInfo &aRxInfo) // only Routers and REEDs respond VerifyOrExit(IsRouterEligible(), error = kErrorInvalidState); - // find MLE Discovery TLV - VerifyOrExit(Tlv::FindTlvOffset(aRxInfo.mMessage, Tlv::kDiscovery, offset) == kErrorNone, error = kErrorParse); - IgnoreError(aRxInfo.mMessage.Read(offset, tlv)); - - offset += sizeof(tlv); - end = offset + sizeof(tlv) + tlv.GetLength(); + SuccessOrExit(error = Tlv::FindTlvValueOffset(aRxInfo.mMessage, Tlv::kDiscovery, offset, length)); + end = offset + length; while (offset < end) { diff --git a/src/core/thread/mle_router.hpp b/src/core/thread/mle_router.hpp index 2eae12a68..811f1a005 100644 --- a/src/core/thread/mle_router.hpp +++ b/src/core/thread/mle_router.hpp @@ -629,7 +629,7 @@ private: void SetStateLeader(uint16_t aRloc16, LeaderStartMode aStartMode); void StopLeader(void); void SynchronizeChildNetworkData(void); - Error UpdateChildAddresses(const Message &aMessage, uint16_t aOffset, Child &aChild); + Error UpdateChildAddresses(const Message &aMessage, uint16_t aOffset, uint16_t aLength, Child &aChild); void UpdateRoutes(const RouteTlv &aRoute, uint8_t aRouterId); bool UpdateLinkQualityOut(const RouteTlv &aRoute, Router &aNeighbor, bool &aResetAdvInterval); bool HasNeighborWithGoodLinkQuality(void) const; diff --git a/src/core/thread/network_data_leader.cpp b/src/core/thread/network_data_leader.cpp index 5b0f785a8..91a7408b5 100644 --- a/src/core/thread/network_data_leader.cpp +++ b/src/core/thread/network_data_leader.cpp @@ -389,18 +389,15 @@ Error LeaderBase::SetNetworkData(uint8_t aVersion, uint8_t aStableVersion, Type aType, const Message &aMessage, - uint16_t aMessageOffset) + uint16_t aOffset, + uint16_t aLength) { - Error error = kErrorNone; - Mle::Tlv tlv; - uint16_t length; + Error error = kErrorNone; - SuccessOrExit(error = aMessage.Read(aMessageOffset, tlv)); + VerifyOrExit(aLength <= kMaxSize, error = kErrorParse); + SuccessOrExit(error = aMessage.Read(aOffset, GetBytes(), aLength)); - length = aMessage.ReadBytes(aMessageOffset + sizeof(tlv), GetBytes(), tlv.GetLength()); - VerifyOrExit(length == tlv.GetLength(), error = kErrorParse); - - SetLength(tlv.GetLength()); + SetLength(static_cast(aLength)); mVersion = aVersion; mStableVersion = aStableVersion; diff --git a/src/core/thread/network_data_leader.hpp b/src/core/thread/network_data_leader.hpp index a38444583..663e5e160 100644 --- a/src/core/thread/network_data_leader.hpp +++ b/src/core/thread/network_data_leader.hpp @@ -143,23 +143,25 @@ public: Error RouteLookup(const Ip6::Address &aSource, const Ip6::Address &aDestination, uint16_t &aRloc16) const; /** - * This method is used by non-Leader devices to set newly received Network Data from the Leader. + * This method is used by non-Leader devices to set Network Data by reading it from a message from Leader. * * @param[in] aVersion The Version value. * @param[in] aStableVersion The Stable Version value. * @param[in] aType The Network Data type to set, the full set or stable subset. - * @param[in] aMessage A reference to the MLE message. - * @param[in] aMessageOffset The offset in @p aMessage for the Network Data TLV. + * @param[in] aMessage A reference to the message. + * @param[in] aOffset The offset in @p aMessage pointing to start of Network Data. + * @param[in] aLength The length of Network Data. * * @retval kErrorNone Successfully set the network data. - * @retval kErrorParse Network Data TLV in @p aMessage is not valid. + * @retval kErrorParse Network Data in @p aMessage is not valid. * */ Error SetNetworkData(uint8_t aVersion, uint8_t aStableVersion, Type aType, const Message &aMessage, - uint16_t aMessageOffset); + uint16_t aOffset, + uint16_t aLength); /** * This method returns a pointer to the Commissioning Data. diff --git a/tests/unit/test_lowpan.cpp b/tests/unit/test_lowpan.cpp index 1f2720d7f..ee4d2ffeb 100644 --- a/tests/unit/test_lowpan.cpp +++ b/tests/unit/test_lowpan.cpp @@ -129,7 +129,8 @@ static void Init(void) SuccessOrQuit(message->AppendBytes(mockNetworkData, sizeof(mockNetworkData))); - IgnoreError(sInstance->Get().SetNetworkData(0, 0, NetworkData::kStableSubset, *message, 0)); + IgnoreError( + sInstance->Get().SetNetworkData(0, 0, NetworkData::kStableSubset, *message, 2, 0x20)); } /**