[tlv] ensure handling of extended TLVs when iterating over sub-TLVs (#10439)

This commit adds the `Tlv::ParseAndSkipTlv()` static method, which
parses a TLV (regular or extended) in a message at a given offset. It
validates that the TLV is fully contained within the message and
updates the offset to skip over the entire parsed TLV.

This helper method is used in various modules where manual iteration
over a sequence of TLVs is performed, specifically to skip over
extended TLVs. The following methods are updated to utilize this new
method and perform additional TLV checks:

- `DiscoverScanner::HandleDiscoveryResponse()`
- `MleRouter::HandleDiscoveryRequest()`
- `LinkMetrics::SubJect::AppendReport()`
- `LinkMetrics::Subject::HandleManagementRequest()`
- `LinkMetrics::Initiator::HandleReport()`
This commit is contained in:
Abtin Keshavarzian
2024-06-26 21:37:01 -04:00
committed by GitHub
parent f2ed3e9c0d
commit 7ca21a38f2
5 changed files with 81 additions and 5 deletions
+18
View File
@@ -56,6 +56,24 @@ const uint8_t *Tlv::GetValue(void) const
Error Tlv::AppendTo(Message &aMessage) const { return aMessage.AppendBytes(this, static_cast<uint16_t>(GetSize())); }
Error Tlv::ParseAndSkipTlv(const Message &aMessage, uint16_t &aOffset)
{
Error error;
ParsedInfo info;
SuccessOrExit(error = info.ParseFrom(aMessage, aOffset));
// `ParseFrom()` has already validated that the entire TLV is
// present within `aMessage`. This ensures that `aOffset + mSize`
// is less than `aMessage.GetLength()`, and therefore we cannot
// have an overflow here.
aOffset += info.mSize;
exit:
return error;
}
Error Tlv::FindTlv(const Message &aMessage, uint8_t aType, uint16_t aMaxSize, Tlv &aTlv)
{
uint16_t offset;
+15
View File
@@ -244,6 +244,21 @@ public:
//------------------------------------------------------------------------------------------------------------------
// Static methods for reading/finding/appending TLVs in a `Message`.
/**
* Parses a TLV in a message at a given offset, validating that it is fully contained within the message and then
* updating the offset to skip over the entire parsed TLV.
*
* Can be used independent of whether the read TLV (from the message) is an Extended TLV or not.
*
* @param[in] aMessage The message to read from.
* @param[in,out] aOffset The offset to read from. On success, it is updated to point after the parsed TLV.
*
* @retval kErrorNone Successfully parsed a TLV from @p aMessage. @p aOffset is updated.
* @retval kErrorParse The TLV was not well-formed or was not fully contained in @p aMessage.
*
*/
static Error ParseAndSkipTlv(const Message &aMessage, uint16_t &aOffset);
/**
* Reads a TLV's value in a message at a given offset expecting a minimum length for the value.
*
+11 -2
View File
@@ -335,12 +335,21 @@ void DiscoverScanner::HandleDiscoveryResponse(Mle::RxInfo &aRxInfo) const
// Process MeshCoP TLVs
while (offset < end)
{
IgnoreError(aRxInfo.mMessage.Read(offset, meshcopTlv));
SuccessOrExit(error = aRxInfo.mMessage.Read(offset, meshcopTlv));
if (meshcopTlv.IsExtended())
{
SuccessOrExit(error = Tlv::ParseAndSkipTlv(aRxInfo.mMessage, offset));
VerifyOrExit(offset <= end, error = kErrorParse);
continue;
}
VerifyOrExit(meshcopTlv.GetSize() + offset <= aRxInfo.mMessage.GetLength(), error = kErrorParse);
switch (meshcopTlv.GetType())
{
case MeshCoP::Tlv::kDiscoveryResponse:
IgnoreError(aRxInfo.mMessage.Read(offset, discoveryResponse));
SuccessOrExit(error = aRxInfo.mMessage.Read(offset, discoveryResponse));
VerifyOrExit(discoveryResponse.IsValid(), error = kErrorParse);
result.mVersion = discoveryResponse.GetVersion();
result.mIsNative = discoveryResponse.IsNativeCommissioner();
+26 -1
View File
@@ -146,7 +146,14 @@ void Initiator::HandleReport(const Message &aMessage, uint16_t aOffset, uint16_t
{
SuccessOrExit(error = aMessage.Read(offset, tlv));
VerifyOrExit(offset + sizeof(Tlv) + tlv.GetLength() <= endOffset, error = kErrorParse);
if (tlv.IsExtended())
{
SuccessOrExit(error = Tlv::ParseAndSkipTlv(aMessage, offset));
VerifyOrExit(offset <= endOffset, error = kErrorParse);
continue;
}
VerifyOrExit(tlv.GetSize() + offset <= endOffset, error = kErrorParse);
// The report must contain either:
// - One or more Report Sub-TLVs (in case of success), or
@@ -320,6 +327,15 @@ Error Initiator::HandleManagementResponse(const Message &aMessage, const Ip6::Ad
SuccessOrExit(error = aMessage.Read(offset, tlv));
if (tlv.IsExtended())
{
SuccessOrExit(error = Tlv::ParseAndSkipTlv(aMessage, offset));
VerifyOrExit(offset <= endOffset, error = kErrorParse);
continue;
}
VerifyOrExit(tlv.GetSize() + offset <= endOffset, error = kErrorParse);
switch (tlv.GetType())
{
case StatusSubTlv::kType:
@@ -440,6 +456,15 @@ Error Subject::AppendReport(Message &aMessage, const Message &aRequestMessage, N
{
SuccessOrExit(error = aRequestMessage.Read(offset, tlv));
if (tlv.IsExtended())
{
SuccessOrExit(error = Tlv::ParseAndSkipTlv(aMessage, offset));
VerifyOrExit(offset <= endOffset, error = kErrorParse);
continue;
}
VerifyOrExit(tlv.GetSize() + offset <= endOffset, error = kErrorParse);
switch (tlv.GetType())
{
case SubTlv::kQueryId:
+11 -2
View File
@@ -2637,12 +2637,21 @@ void MleRouter::HandleDiscoveryRequest(RxInfo &aRxInfo)
while (offset < end)
{
IgnoreError(aRxInfo.mMessage.Read(offset, meshcopTlv));
SuccessOrExit(error = aRxInfo.mMessage.Read(offset, meshcopTlv));
if (meshcopTlv.IsExtended())
{
SuccessOrExit(error = Tlv::ParseAndSkipTlv(aRxInfo.mMessage, offset));
VerifyOrExit(offset <= end, error = kErrorParse);
continue;
}
VerifyOrExit(meshcopTlv.GetSize() + offset <= aRxInfo.mMessage.GetLength(), error = kErrorParse);
switch (meshcopTlv.GetType())
{
case MeshCoP::Tlv::kDiscoveryRequest:
IgnoreError(aRxInfo.mMessage.Read(offset, discoveryRequestTlv));
SuccessOrExit(error = aRxInfo.mMessage.Read(offset, discoveryRequestTlv));
VerifyOrExit(discoveryRequestTlv.IsValid(), error = kErrorParse);
break;