From 7cc5ba1358f23eac4bc97a152625f72f16bd4cea Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Fri, 1 Sep 2017 00:23:39 -0700 Subject: [PATCH] [network-data] add additional TLV length checks (#2154) Credit to OSS-Fuzz. --- src/core/thread/network_data.cpp | 17 +++++++++++++ src/core/thread/network_data_leader_ftd.cpp | 28 +++++++++++++++++---- src/core/thread/network_data_leader_ftd.hpp | 2 +- 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/core/thread/network_data.cpp b/src/core/thread/network_data.cpp index 50a85507e..0813c7c1a 100644 --- a/src/core/thread/network_data.cpp +++ b/src/core/thread/network_data.cpp @@ -104,6 +104,8 @@ otError NetworkData::GetNextOnMeshPrefix(otNetworkDataIterator *aIterator, uint1 BorderRouterTlv *borderRouter; BorderRouterEntry *borderRouterEntry = NULL; + VerifyOrExit(cur + sizeof(NetworkDataTlv) <= end && cur->GetNext() <= end, error = OT_ERROR_PARSE); + if (cur->GetType() != NetworkDataTlv::kTypePrefix) { continue; @@ -172,6 +174,8 @@ otError NetworkData::GetNextExternalRoute(otNetworkDataIterator *aIterator, uint HasRouteTlv *hasRoute; HasRouteEntry *hasRouteEntry = NULL; + VerifyOrExit(cur + sizeof(NetworkDataTlv) <= end && cur->GetNext() <= end, error = OT_ERROR_PARSE); + if (cur->GetType() != NetworkDataTlv::kTypePrefix) { continue; @@ -438,6 +442,8 @@ BorderRouterTlv *NetworkData::FindBorderRouter(PrefixTlv &aPrefix) while (cur < end) { + VerifyOrExit(cur + sizeof(NetworkDataTlv) <= end && cur->GetNext() <= end); + if (cur->GetType() == NetworkDataTlv::kTypeBorderRouter) { ExitNow(rval = reinterpret_cast(cur)); @@ -458,6 +464,8 @@ BorderRouterTlv *NetworkData::FindBorderRouter(PrefixTlv &aPrefix, bool aStable) while (cur < end) { + VerifyOrExit(cur + sizeof(NetworkDataTlv) <= end && cur->GetNext() <= end); + if (cur->GetType() == NetworkDataTlv::kTypeBorderRouter && cur->IsStable() == aStable) { @@ -479,6 +487,8 @@ HasRouteTlv *NetworkData::FindHasRoute(PrefixTlv &aPrefix) while (cur < end) { + VerifyOrExit(cur + sizeof(NetworkDataTlv) <= end && cur->GetNext() <= end); + if (cur->GetType() == NetworkDataTlv::kTypeHasRoute) { ExitNow(rval = reinterpret_cast(cur)); @@ -499,6 +509,8 @@ HasRouteTlv *NetworkData::FindHasRoute(PrefixTlv &aPrefix, bool aStable) while (cur < end) { + VerifyOrExit(cur + sizeof(NetworkDataTlv) <= end && cur->GetNext() <= end); + if (cur->GetType() == NetworkDataTlv::kTypeHasRoute && cur->IsStable() == aStable) { @@ -520,6 +532,8 @@ ContextTlv *NetworkData::FindContext(PrefixTlv &aPrefix) while (cur < end) { + VerifyOrExit(cur + sizeof(NetworkDataTlv) <= end && cur->GetNext() <= end); + if (cur->GetType() == NetworkDataTlv::kTypeContext) { ExitNow(rval = reinterpret_cast(cur)); @@ -545,6 +559,8 @@ PrefixTlv *NetworkData::FindPrefix(const uint8_t *aPrefix, uint8_t aPrefixLength while (cur < end) { + VerifyOrExit(cur + sizeof(NetworkDataTlv) <= end && cur->GetNext() <= end); + if (cur->GetType() == NetworkDataTlv::kTypePrefix) { compare = reinterpret_cast(cur); @@ -559,6 +575,7 @@ PrefixTlv *NetworkData::FindPrefix(const uint8_t *aPrefix, uint8_t aPrefixLength cur = cur->GetNext(); } +exit: return NULL; } diff --git a/src/core/thread/network_data_leader_ftd.cpp b/src/core/thread/network_data_leader_ftd.cpp index ff8ef4406..543b5b252 100644 --- a/src/core/thread/network_data_leader_ftd.cpp +++ b/src/core/thread/network_data_leader_ftd.cpp @@ -414,8 +414,9 @@ exit: } } -void Leader::RlocLookup(uint16_t aRloc16, bool &aIn, bool &aStable, uint8_t *aTlvs, uint8_t aTlvsLength) +otError Leader::RlocLookup(uint16_t aRloc16, bool &aIn, bool &aStable, uint8_t *aTlvs, uint8_t aTlvsLength) { + otError error = OT_ERROR_NONE; NetworkDataTlv *cur = reinterpret_cast(aTlvs); NetworkDataTlv *end = reinterpret_cast(aTlvs + aTlvsLength); NetworkDataTlv *subCur; @@ -428,14 +429,21 @@ void Leader::RlocLookup(uint16_t aRloc16, bool &aIn, bool &aStable, uint8_t *aTl while (cur < end) { + VerifyOrExit(cur + sizeof(NetworkDataTlv) <= end && cur->GetNext() <= end, error = OT_ERROR_PARSE); + if (cur->GetType() == NetworkDataTlv::kTypePrefix) { prefix = static_cast(cur); subCur = prefix->GetSubTlvs(); subEnd = prefix->GetNext(); + VerifyOrExit(subEnd <= end, error = OT_ERROR_PARSE); + while (subCur < subEnd) { + VerifyOrExit(subCur + sizeof(NetworkDataTlv) <= subEnd && subCur->GetNext() <= subEnd, + error = OT_ERROR_PARSE); + switch (subCur->GetType()) { case NetworkDataTlv::kTypeBorderRouter: @@ -495,7 +503,7 @@ void Leader::RlocLookup(uint16_t aRloc16, bool &aIn, bool &aStable, uint8_t *aTl } exit: - return; + return error; } bool Leader::IsStableUpdated(uint16_t aRloc16, uint8_t *aTlvs, uint8_t aTlvsLength, uint8_t *aTlvsBase, @@ -507,6 +515,8 @@ bool Leader::IsStableUpdated(uint16_t aRloc16, uint8_t *aTlvs, uint8_t aTlvsLeng while (cur < end) { + VerifyOrExit(cur + sizeof(NetworkDataTlv) <= end && cur->GetNext() <= end); + if (cur->GetType() == NetworkDataTlv::kTypePrefix) { PrefixTlv *prefix = static_cast(cur); @@ -583,7 +593,7 @@ otError Leader::RegisterNetworkData(uint16_t aRloc16, uint8_t *aTlvs, uint8_t aT } else { - RlocLookup(aRloc16, rlocIn, rlocStable, aTlvs, aTlvsLength); + SuccessOrExit(error = RlocLookup(aRloc16, rlocIn, rlocStable, aTlvs, aTlvsLength)); SuccessOrExit(error = AddNetworkData(aTlvs, aTlvsLength)); mVersion++; @@ -602,11 +612,14 @@ exit: otError Leader::AddNetworkData(uint8_t *aTlvs, uint8_t aTlvsLength) { + otError error = OT_ERROR_NONE; NetworkDataTlv *cur = reinterpret_cast(aTlvs); NetworkDataTlv *end = reinterpret_cast(aTlvs + aTlvsLength); while (cur < end) { + VerifyOrExit(cur + sizeof(NetworkDataTlv) <= end && cur->GetNext() <= end, error = OT_ERROR_NONE); + switch (cur->GetType()) { case NetworkDataTlv::kTypePrefix: @@ -623,16 +636,20 @@ otError Leader::AddNetworkData(uint8_t *aTlvs, uint8_t aTlvsLength) otDumpDebgNetData(GetInstance(), "add done", mTlvs, mLength); - return OT_ERROR_NONE; +exit: + return error; } otError Leader::AddPrefix(PrefixTlv &aPrefix) { + otError error = OT_ERROR_NONE; NetworkDataTlv *cur = aPrefix.GetSubTlvs(); NetworkDataTlv *end = aPrefix.GetNext(); while (cur < end) { + VerifyOrExit(cur + sizeof(NetworkDataTlv) <= end && cur->GetNext() <= end, error = OT_ERROR_NONE); + switch (cur->GetType()) { case NetworkDataTlv::kTypeHasRoute: @@ -650,7 +667,8 @@ otError Leader::AddPrefix(PrefixTlv &aPrefix) cur = cur->GetNext(); } - return OT_ERROR_NONE; +exit: + return error; } otError Leader::AddHasRoute(PrefixTlv &aPrefix, HasRouteTlv &aHasRoute) diff --git a/src/core/thread/network_data_leader_ftd.hpp b/src/core/thread/network_data_leader_ftd.hpp index 483b0f56c..4340f51fc 100644 --- a/src/core/thread/network_data_leader_ftd.hpp +++ b/src/core/thread/network_data_leader_ftd.hpp @@ -168,7 +168,7 @@ private: otError RemoveRloc(PrefixTlv &aPrefix, HasRouteTlv &aHasRoute, uint16_t aRloc16); otError RemoveRloc(PrefixTlv &aPrefix, BorderRouterTlv &aBorderRouter, uint16_t aRloc16); - void RlocLookup(uint16_t aRloc16, bool &aIn, bool &aStable, uint8_t *aTlvs, uint8_t aTlvsLength); + otError RlocLookup(uint16_t aRloc16, bool &aIn, bool &aStable, uint8_t *aTlvs, uint8_t aTlvsLength); bool IsStableUpdated(uint16_t aRloc16, uint8_t *aTlvs, uint8_t aTlvsLength, uint8_t *aTlvsBase, uint8_t aTlvsBaseLength);