[mle] reject `ChildIdResponse` if included dataset TLV is ill-formed (#7599)
This commit contains the following changes:
- The `Dataset` method which reads the dataset TLVs from a message
is renamed to `ReadFromMessage()` and after reading the TLVs, it
also checks that the TLVs in the Dataset are valid (well-formed).
- The `Active/PendingDataset::Save()` methods are updated to return
error in the case the parsed dataset is not well-formed.
- The `Mle::HandleChildIdRequest()` rejects the response if the
included dataset is not well-formed. This ensures that device
stays detached if it cannot accept and save the dataset it gets
from the parent candidate.
diff --git a/src/core/meshcop/dataset.cpp b/src/core/meshcop/dataset.cpp
index b2fea0d..b6496f2 100644
--- a/src/core/meshcop/dataset.cpp
+++ b/src/core/meshcop/dataset.cpp
@@ -435,13 +435,15 @@
return SetTlv(aTlv.GetType(), aTlv.GetValue(), aTlv.GetLength());
}
-Error Dataset::Set(const Message &aMessage, uint16_t aOffset, uint8_t aLength)
+Error Dataset::ReadFromMessage(const Message &aMessage, uint16_t aOffset, uint8_t aLength)
{
- Error error = kErrorInvalidArgs;
+ Error error = kErrorParse;
SuccessOrExit(aMessage.Read(aOffset, mTlvs, aLength));
mLength = aLength;
+ VerifyOrExit(IsValid(), error = kErrorParse);
+
mUpdateTime = TimerMilli::GetNow();
error = kErrorNone;
diff --git a/src/core/meshcop/dataset.hpp b/src/core/meshcop/dataset.hpp
index 0e2f080..353f1af 100644
--- a/src/core/meshcop/dataset.hpp
+++ b/src/core/meshcop/dataset.hpp
@@ -782,17 +782,17 @@
}
/**
- * This method sets the Dataset using TLVs stored in a message buffer.
+ * This method reads the Dataset from a given message and checks that it is well-formed and valid.
*
- * @param[in] aMessage The message buffer.
- * @param[in] aOffset The message buffer offset where the dataset starts.
- * @param[in] aLength The TLVs length in the message buffer in bytes.
+ * @param[in] aMessage The message to read from.
+ * @param[in] aOffset The offset in @p aMessage to start reading the Dataset TLVs.
+ * @param[in] aLength The dataset length in bytes.
*
- * @retval kErrorNone Successfully set the Dataset.
- * @retval kErrorInvalidArgs The values of @p aOffset and @p aLength are not valid for @p aMessage.
+ * @retval kErrorNone Successfully read and validated the Dataset.
+ * @retval kErrorParse Could not read or parse the dataset from @p aMessage.
*
*/
- Error Set(const Message &aMessage, uint16_t aOffset, uint8_t aLength);
+ Error ReadFromMessage(const Message &aMessage, uint16_t aOffset, uint8_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 b1e46eb..1c00ea8 100644
--- a/src/core/meshcop/dataset_manager.cpp
+++ b/src/core/meshcop/dataset_manager.cpp
@@ -140,7 +140,7 @@
if (isNetworkkeyUpdated || compare > 0)
{
- IgnoreError(mLocal.Save(aDataset));
+ SuccessOrExit(error = mLocal.Save(aDataset));
#if OPENTHREAD_FTD
Get<NetworkData::Leader>().IncrementVersionAndStableVersion();
@@ -690,9 +690,9 @@
Error error = kErrorNone;
Dataset dataset;
- SuccessOrExit(error = dataset.Set(aMessage, aOffset, aLength));
+ SuccessOrExit(error = dataset.ReadFromMessage(aMessage, aOffset, aLength));
dataset.SetTimestamp(Dataset::kActive, aTimestamp);
- IgnoreError(DatasetManager::Save(dataset));
+ error = DatasetManager::Save(dataset);
exit:
return error;
@@ -777,9 +777,9 @@
Error error = kErrorNone;
Dataset dataset;
- SuccessOrExit(error = dataset.Set(aMessage, aOffset, aLength));
+ SuccessOrExit(error = dataset.ReadFromMessage(aMessage, aOffset, aLength));
dataset.SetTimestamp(Dataset::kPending, aTimestamp);
- IgnoreError(DatasetManager::Save(dataset));
+ SuccessOrExit(error = DatasetManager::Save(dataset));
StartDelayTimer();
exit:
diff --git a/src/core/meshcop/dataset_manager.hpp b/src/core/meshcop/dataset_manager.hpp
index 75ee9f7..ec7c316 100644
--- a/src/core/meshcop/dataset_manager.hpp
+++ b/src/core/meshcop/dataset_manager.hpp
@@ -272,6 +272,9 @@
* @param[in] aOffset The offset where the Operational Dataset begins.
* @param[in] aLength The length of the Operational Dataset.
*
+ * @retval kErrorNone Successfully parsed the Dataset from the @p aMessage and saved it.
+ * @retval kErrorParse Could not parse the Dataset from @p aMessage.
+ *
*/
Error Save(const Timestamp &aTimestamp, const Message &aMessage, uint16_t aOffset, uint8_t aLength);
@@ -419,6 +422,9 @@
* @param[in] aOffset The offset where the Operational Dataset begins.
* @param[in] aLength The length of the Operational Dataset.
*
+ * @retval kErrorNone Successfully parsed the Dataset from the @p aMessage and saved it.
+ * @retval kErrorParse Could not parse the Dataset from @p aMessage.
+ *
*/
Error Save(const Timestamp &aTimestamp, const Message &aMessage, uint16_t aOffset, uint8_t aLength);
diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp
index cff7228..467328e 100644
--- a/src/core/thread/mle.cpp
+++ b/src/core/thread/mle.cpp
@@ -3696,7 +3696,8 @@
if (Tlv::FindTlvOffset(aMessage, Tlv::kActiveDataset, offset) == kErrorNone)
{
IgnoreError(aMessage.Read(offset, tlv));
- IgnoreError(Get<MeshCoP::ActiveDataset>().Save(timestamp, aMessage, offset + sizeof(tlv), tlv.GetLength()));
+ SuccessOrExit(
+ error = Get<MeshCoP::ActiveDataset>().Save(timestamp, aMessage, offset + sizeof(tlv), tlv.GetLength()));
}
break;
