From de8033fad43cdec70ed8f636b4bfb1c2cc1d2071 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Thu, 24 Oct 2019 23:03:54 -0700 Subject: [PATCH] [mle] fix MleRouter::HandleChildUpdateResponse msg validation (#4297) This commit moves the message security check to the top of MleRouter::HandleChildUpdateResponse. --- src/core/thread/mle.cpp | 2 +- src/core/thread/mle_router.cpp | 42 +++++++++++++++--------------- src/core/thread/mle_router_ftd.hpp | 3 ++- src/core/thread/mle_router_mtd.hpp | 5 +++- 4 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp index d88434015..d2536f5e3 100644 --- a/src/core/thread/mle.cpp +++ b/src/core/thread/mle.cpp @@ -2820,7 +2820,7 @@ void Mle::HandleUdpReceive(Message &aMessage, const Ip6::MessageInfo &aMessageIn case Header::kCommandChildUpdateResponse: if (mRole == OT_DEVICE_ROLE_LEADER || mRole == OT_DEVICE_ROLE_ROUTER) { - Get().HandleChildUpdateResponse(aMessage, aMessageInfo, keySequence); + Get().HandleChildUpdateResponse(aMessage, aMessageInfo, keySequence, neighbor); } else { diff --git a/src/core/thread/mle_router.cpp b/src/core/thread/mle_router.cpp index ef6fa9697..6975d1a74 100644 --- a/src/core/thread/mle_router.cpp +++ b/src/core/thread/mle_router.cpp @@ -2370,11 +2370,11 @@ exit: otError MleRouter::HandleChildUpdateResponse(const Message & aMessage, const Ip6::MessageInfo &aMessageInfo, - uint32_t aKeySequence) + uint32_t aKeySequence, + Neighbor * aNeighbor) { otError error = OT_ERROR_NONE; const otThreadLinkInfo *linkInfo = static_cast(aMessageInfo.GetLinkInfo()); - Mac::ExtAddress macAddr; SourceAddressTlv sourceAddress; TimeoutTlv timeout; ResponseTlv response; @@ -2385,17 +2385,28 @@ otError MleRouter::HandleChildUpdateResponse(const Message & aMessage, Child * child; uint16_t addressRegistrationOffset = 0; - // Find Child - aMessageInfo.GetPeerAddr().ToExtAddress(macAddr); - - child = mChildTable.FindChild(macAddr, ChildTable::kInStateAnyExceptInvalid); - - if (child == NULL) + if ((aNeighbor == NULL) || IsActiveRouter(aNeighbor->GetRloc16())) { LogMleMessage("Receive Child Update Response from unknown child", aMessageInfo.GetPeerAddr()); ExitNow(error = OT_ERROR_NOT_FOUND); } + child = static_cast(aNeighbor); + + // Response + if (Tlv::GetTlv(aMessage, Tlv::kResponse, sizeof(response), response) == OT_ERROR_NONE) + { + VerifyOrExit(response.IsValid() && + memcmp(response.GetResponse(), child->GetChallenge(), child->GetChallengeSize()) == 0, + error = OT_ERROR_SECURITY); + } + else + { + VerifyOrExit(child->IsStateValid(), error = OT_ERROR_SECURITY); + } + + LogMleMessage("Receive Child Update Response from child", aMessageInfo.GetPeerAddr(), child->GetRloc16()); + // Source Address if (Tlv::GetTlv(aMessage, Tlv::kSourceAddress, sizeof(sourceAddress), sourceAddress) == OT_ERROR_NONE) { @@ -2403,22 +2414,11 @@ otError MleRouter::HandleChildUpdateResponse(const Message & aMessage, if (child->GetRloc16() != sourceAddress.GetRloc16()) { - LogMleMessage("Receive Child Update Response from unknown child", aMessageInfo.GetPeerAddr(), - sourceAddress.GetRloc16()); - ExitNow(error = OT_ERROR_PARSE); + RemoveNeighbor(*child); + ExitNow(); } } - LogMleMessage("Receive Child Update Response from child", aMessageInfo.GetPeerAddr(), child->GetRloc16()); - - // Response - if (Tlv::GetTlv(aMessage, Tlv::kResponse, sizeof(response), response) == OT_ERROR_NONE) - { - VerifyOrExit(response.IsValid() && - memcmp(response.GetResponse(), child->GetChallenge(), child->GetChallengeSize()) == 0, - error = OT_ERROR_SECURITY); - } - // Status if (Tlv::GetTlv(aMessage, Tlv::kStatus, sizeof(status), status) == OT_ERROR_NONE) { diff --git a/src/core/thread/mle_router_ftd.hpp b/src/core/thread/mle_router_ftd.hpp index d9acdfd8b..dd36e85dd 100644 --- a/src/core/thread/mle_router_ftd.hpp +++ b/src/core/thread/mle_router_ftd.hpp @@ -691,7 +691,8 @@ private: uint32_t aKeySequence); otError HandleChildUpdateResponse(const Message & aMessage, const Ip6::MessageInfo &aMessageInfo, - uint32_t aKeySequence); + uint32_t aKeySequence, + Neighbor * aNeighbor); otError HandleDataRequest(const Message &aMessage, const Ip6::MessageInfo &aMessageInfo); void HandleNetworkDataUpdateRouter(void); otError HandleDiscoveryRequest(const Message &aMessage, const Ip6::MessageInfo &aMessageInfo); diff --git a/src/core/thread/mle_router_mtd.hpp b/src/core/thread/mle_router_mtd.hpp index 68cc29d65..65492677c 100644 --- a/src/core/thread/mle_router_mtd.hpp +++ b/src/core/thread/mle_router_mtd.hpp @@ -137,7 +137,10 @@ private: otError HandleParentRequest(const Message &, const Ip6::MessageInfo &) { return OT_ERROR_DROP; } otError HandleChildIdRequest(const Message &, const Ip6::MessageInfo &, uint32_t) { return OT_ERROR_DROP; } otError HandleChildUpdateRequest(const Message &, const Ip6::MessageInfo &, uint32_t) { return OT_ERROR_DROP; } - otError HandleChildUpdateResponse(const Message &, const Ip6::MessageInfo &, uint32_t) { return OT_ERROR_DROP; } + otError HandleChildUpdateResponse(const Message &, const Ip6::MessageInfo &, uint32_t, Neighbor *) + { + return OT_ERROR_DROP; + } otError HandleDataRequest(const Message &, const Ip6::MessageInfo &) { return OT_ERROR_DROP; } void HandleNetworkDataUpdateRouter(void) {} otError HandleDiscoveryRequest(const Message &, const Ip6::MessageInfo &) { return OT_ERROR_DROP; }