From d85bb5a2c10a2f10622722020d68516e5f3c4c17 Mon Sep 17 00:00:00 2001 From: Kamil Burzynski Date: Thu, 24 Aug 2017 20:24:51 +0200 Subject: [PATCH] [network-data] add protection against memcpy() memory overwrite (#2125) --- src/core/api/border_router_api.cpp | 2 +- src/core/api/netdata_api.cpp | 2 +- src/core/thread/mle.cpp | 4 +++- src/core/thread/network_data.cpp | 8 +++++++- src/core/thread/network_data.hpp | 6 +++++- 5 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/core/api/border_router_api.cpp b/src/core/api/border_router_api.cpp index 8a2c46a5b..3bc0feddc 100644 --- a/src/core/api/border_router_api.cpp +++ b/src/core/api/border_router_api.cpp @@ -47,7 +47,7 @@ otError otBorderRouterGetNetData(otInstance *aInstance, bool aStable, uint8_t *a VerifyOrExit(aData != NULL && aDataLength != NULL, error = OT_ERROR_INVALID_ARGS); - aInstance->mThreadNetif.GetNetworkDataLocal().GetNetworkData(aStable, aData, *aDataLength); + error = aInstance->mThreadNetif.GetNetworkDataLocal().GetNetworkData(aStable, aData, *aDataLength); exit: return error; diff --git a/src/core/api/netdata_api.cpp b/src/core/api/netdata_api.cpp index 3ab370ca5..6378f1986 100644 --- a/src/core/api/netdata_api.cpp +++ b/src/core/api/netdata_api.cpp @@ -44,7 +44,7 @@ otError otNetDataGet(otInstance *aInstance, bool aStable, uint8_t *aData, uint8_ VerifyOrExit(aData != NULL && aDataLength != NULL, error = OT_ERROR_INVALID_ARGS); - aInstance->mThreadNetif.GetNetworkDataLeader().GetNetworkData(aStable, aData, *aDataLength); + error = aInstance->mThreadNetif.GetNetworkDataLeader().GetNetworkData(aStable, aData, *aDataLength); exit: return error; diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp index 168bb0c38..33595f187 100644 --- a/src/core/thread/mle.cpp +++ b/src/core/thread/mle.cpp @@ -1061,7 +1061,9 @@ otError Mle::AppendLeaderData(Message &aMessage) void Mle::FillNetworkDataTlv(NetworkDataTlv &aTlv, bool aStableOnly) { - uint8_t length; + uint8_t length = sizeof(NetworkDataTlv) - sizeof(Tlv); // sizeof( NetworkDataTlv::mNetworkData ) + + // Ignore result code, provided buffer must be enough GetNetif().GetNetworkDataLeader().GetNetworkData(aStableOnly, aTlv.GetNetworkData(), length); aTlv.SetLength(length); } diff --git a/src/core/thread/network_data.cpp b/src/core/thread/network_data.cpp index 5f11d7170..50a85507e 100644 --- a/src/core/thread/network_data.cpp +++ b/src/core/thread/network_data.cpp @@ -66,9 +66,12 @@ void NetworkData::Clear(void) mLength = 0; } -void NetworkData::GetNetworkData(bool aStable, uint8_t *aData, uint8_t &aDataLength) +otError NetworkData::GetNetworkData(bool aStable, uint8_t *aData, uint8_t &aDataLength) { + otError error = OT_ERROR_NONE; + assert(aData != NULL); + VerifyOrExit(aDataLength >= mLength, error = OT_ERROR_NO_BUFS); memcpy(aData, mTlvs, mLength); aDataLength = mLength; @@ -77,6 +80,9 @@ void NetworkData::GetNetworkData(bool aStable, uint8_t *aData, uint8_t &aDataLen { RemoveTemporaryData(aData, aDataLength); } + +exit: + return error; } otError NetworkData::GetNextOnMeshPrefix(otNetworkDataIterator *aIterator, otBorderRouterConfig *aConfig) diff --git a/src/core/thread/network_data.hpp b/src/core/thread/network_data.hpp index 8df508143..972a76f56 100644 --- a/src/core/thread/network_data.hpp +++ b/src/core/thread/network_data.hpp @@ -114,8 +114,12 @@ public: * @param[out] aData A pointer to the data buffer. * @param[inout] aDataLength On entry, size of the data buffer pointed to by @p aData. * On exit, number of copied bytes. + * + * @retval OT_ERROR_NONE Successfully copied full Thread Network Data. + * @retval OT_ERROR_NO_BUFS Not enough space to fully copy Thread Network Data. + * */ - void GetNetworkData(bool aStable, uint8_t *aData, uint8_t &aDataLength); + otError GetNetworkData(bool aStable, uint8_t *aData, uint8_t &aDataLength); /** * This method provides the next On Mesh prefix in the Thread Network Data.