Skip to content

Commit 4722019

Browse files
yunhanw-googlepull[bot]
yunhanw-google
authored and
pull[bot]
committed
Add strict IM encoding ordering check (#12549)
1 parent cf1550f commit 4722019

13 files changed

+127
-82
lines changed

‎src/app/EventManagement.cpp

+5-7
Original file line numberDiff line numberDiff line change
@@ -449,12 +449,6 @@ CHIP_ERROR EventManagement::CopyAndAdjustDeltaTime(const TLVReader & aReader, si
449449
CopyAndAdjustDeltaTimeContext * ctx = static_cast<CopyAndAdjustDeltaTimeContext *>(apContext);
450450
TLVReader reader(aReader);
451451

452-
if (aReader.GetTag() == TLV::ContextTag(to_underlying(EventDataIB::Tag::kPath)))
453-
{
454-
err =
455-
ctx->mpWriter->Put(TLV::ContextTag(to_underlying(EventDataIB::Tag::kEventNumber)), ctx->mpContext->mCurrentEventNumber);
456-
}
457-
458452
if (aReader.GetTag() == TLV::ContextTag(to_underlying(EventDataIB::Tag::kDeltaSystemTimestamp)) ||
459453
aReader.GetTag() == TLV::ContextTag(to_underlying(EventDataIB::Tag::kDeltaEpochTimestamp)))
460454
{
@@ -490,6 +484,11 @@ CHIP_ERROR EventManagement::CopyAndAdjustDeltaTime(const TLVReader & aReader, si
490484
err = ctx->mpWriter->CopyElement(reader);
491485
}
492486

487+
if (aReader.GetTag() == TLV::ContextTag(to_underlying(EventDataIB::Tag::kPath)))
488+
{
489+
err =
490+
ctx->mpWriter->Put(TLV::ContextTag(to_underlying(EventDataIB::Tag::kEventNumber)), ctx->mpContext->mCurrentEventNumber);
491+
}
493492
return err;
494493
}
495494

@@ -762,7 +761,6 @@ CHIP_ERROR EventManagement::FetchEventsSince(TLVWriter & aWriter, ClusterInfo *
762761
}
763762

764763
exit:
765-
ChipLogProgress(EventLogging, "Debug log, err: %s\n", chip::ErrorStr(err));
766764
aEventNumber = context.mCurrentEventNumber;
767765
aEventCount += context.mEventCount;
768766
return err;

‎src/app/MessageDef/ReportDataMessage.h

+5-5
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,11 @@ namespace app {
4040
namespace ReportDataMessage {
4141
enum
4242
{
43-
kCsTag_SuppressResponse = 0,
44-
kCsTag_SubscriptionId = 1,
45-
kCsTag_AttributeReportIBs = 2,
46-
kCsTag_EventReports = 3,
47-
kCsTag_MoreChunkedMessages = 4,
43+
kCsTag_SubscriptionId = 0,
44+
kCsTag_AttributeReportIBs = 1,
45+
kCsTag_EventReports = 2,
46+
kCsTag_MoreChunkedMessages = 3,
47+
kCsTag_SuppressResponse = 4,
4848
};
4949

5050
class Parser : public StructParser

‎src/app/MessageDef/StructParser.cpp

+30
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,37 @@ CHIP_ERROR StructParser::Init(const TLV::TLVReader & aReader)
2323
mReader.Init(aReader);
2424
VerifyOrReturnError(TLV::kTLVType_Structure == mReader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE);
2525
ReturnErrorOnFailure(mReader.EnterContainer(mOuterContainerType));
26+
ReturnErrorOnFailure(CheckSchemaOrdering());
2627
return CHIP_NO_ERROR;
2728
}
29+
30+
CHIP_ERROR StructParser::CheckSchemaOrdering() const
31+
{
32+
CHIP_ERROR err = CHIP_NO_ERROR;
33+
TLV::TLVReader reader;
34+
reader.Init(mReader);
35+
uint32_t preTagNum = 0;
36+
bool first = true;
37+
while (CHIP_NO_ERROR == (err = reader.Next()))
38+
{
39+
VerifyOrReturnError(TLV::IsContextTag(reader.GetTag()), CHIP_ERROR_INVALID_TLV_TAG);
40+
uint32_t tagNum = TLV::TagNumFromTag(reader.GetTag());
41+
if (first || (preTagNum < tagNum))
42+
{
43+
preTagNum = tagNum;
44+
}
45+
else
46+
{
47+
return CHIP_ERROR_INVALID_TLV_TAG;
48+
}
49+
first = false;
50+
}
51+
if (CHIP_END_OF_TLV == err)
52+
{
53+
err = CHIP_NO_ERROR;
54+
}
55+
ReturnErrorOnFailure(err);
56+
return reader.ExitContainer(mOuterContainerType);
57+
}
2858
} // namespace app
2959
} // namespace chip

‎src/app/MessageDef/StructParser.h

+2
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ class StructParser : public Parser
3333
* @return #CHIP_NO_ERROR on success
3434
*/
3535
CHIP_ERROR Init(const TLV::TLVReader & aReader);
36+
37+
CHIP_ERROR CheckSchemaOrdering() const;
3638
};
3739
} // namespace app
3840
} // namespace chip

‎src/app/ReadClient.cpp

+25-25
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,15 @@ CHIP_ERROR ReadClient::SendReadRequest(ReadPrepareParams & aReadPrepareParams)
140140
err = request.Init(&writer);
141141
SuccessOrExit(err);
142142

143+
if (aReadPrepareParams.mAttributePathParamsListSize != 0 && aReadPrepareParams.mpAttributePathParamsList != nullptr)
144+
{
145+
AttributePathIBs::Builder attributePathListBuilder = request.CreateAttributeRequests();
146+
SuccessOrExit(err = attributePathListBuilder.GetError());
147+
err = GenerateAttributePathList(attributePathListBuilder, aReadPrepareParams.mpAttributePathParamsList,
148+
aReadPrepareParams.mAttributePathParamsListSize);
149+
SuccessOrExit(err);
150+
}
151+
143152
if (aReadPrepareParams.mEventPathParamsListSize != 0 && aReadPrepareParams.mpEventPathParamsList != nullptr)
144153
{
145154
EventPathIBs::Builder & eventPathListBuilder = request.CreateEventRequests();
@@ -160,15 +169,6 @@ CHIP_ERROR ReadClient::SendReadRequest(ReadPrepareParams & aReadPrepareParams)
160169
}
161170
}
162171

163-
if (aReadPrepareParams.mAttributePathParamsListSize != 0 && aReadPrepareParams.mpAttributePathParamsList != nullptr)
164-
{
165-
AttributePathIBs::Builder attributePathListBuilder = request.CreateAttributeRequests();
166-
SuccessOrExit(err = attributePathListBuilder.GetError());
167-
err = GenerateAttributePathList(attributePathListBuilder, aReadPrepareParams.mpAttributePathParamsList,
168-
aReadPrepareParams.mAttributePathParamsListSize);
169-
SuccessOrExit(err);
170-
}
171-
172172
request.IsFabricFiltered(false).EndOfReadRequestMessage();
173173
SuccessOrExit(err = request.GetError());
174174

@@ -632,12 +632,27 @@ CHIP_ERROR ReadClient::SendSubscribeRequest(ReadPrepareParams & aReadPreparePara
632632
VerifyOrExit(mpCallback != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
633633
msgBuf = System::PacketBufferHandle::New(kMaxSecureSduLengthBytes);
634634
VerifyOrExit(!msgBuf.IsNull(), err = CHIP_ERROR_NO_MEMORY);
635+
VerifyOrExit(aReadPrepareParams.mMinIntervalFloorSeconds < aReadPrepareParams.mMaxIntervalCeilingSeconds,
636+
err = CHIP_ERROR_INVALID_ARGUMENT);
635637

636638
writer.Init(std::move(msgBuf));
637639

638640
err = request.Init(&writer);
639641
SuccessOrExit(err);
640642

643+
request.KeepSubscriptions(aReadPrepareParams.mKeepSubscriptions)
644+
.MinIntervalFloorSeconds(aReadPrepareParams.mMinIntervalFloorSeconds)
645+
.MaxIntervalCeilingSeconds(aReadPrepareParams.mMaxIntervalCeilingSeconds);
646+
647+
if (aReadPrepareParams.mAttributePathParamsListSize != 0 && aReadPrepareParams.mpAttributePathParamsList != nullptr)
648+
{
649+
AttributePathIBs::Builder & attributePathListBuilder = request.CreateAttributeRequests();
650+
SuccessOrExit(err = attributePathListBuilder.GetError());
651+
err = GenerateAttributePathList(attributePathListBuilder, aReadPrepareParams.mpAttributePathParamsList,
652+
aReadPrepareParams.mAttributePathParamsListSize);
653+
SuccessOrExit(err);
654+
}
655+
641656
if (aReadPrepareParams.mEventPathParamsListSize != 0 && aReadPrepareParams.mpEventPathParamsList != nullptr)
642657
{
643658
EventPathIBs::Builder & eventPathListBuilder = request.CreateEventRequests();
@@ -659,22 +674,7 @@ CHIP_ERROR ReadClient::SendSubscribeRequest(ReadPrepareParams & aReadPreparePara
659674
}
660675
}
661676

662-
if (aReadPrepareParams.mAttributePathParamsListSize != 0 && aReadPrepareParams.mpAttributePathParamsList != nullptr)
663-
{
664-
AttributePathIBs::Builder & attributePathListBuilder = request.CreateAttributeRequests();
665-
SuccessOrExit(err = attributePathListBuilder.GetError());
666-
err = GenerateAttributePathList(attributePathListBuilder, aReadPrepareParams.mpAttributePathParamsList,
667-
aReadPrepareParams.mAttributePathParamsListSize);
668-
SuccessOrExit(err);
669-
}
670-
671-
VerifyOrExit(aReadPrepareParams.mMinIntervalFloorSeconds < aReadPrepareParams.mMaxIntervalCeilingSeconds,
672-
err = CHIP_ERROR_INVALID_ARGUMENT);
673-
request.MinIntervalFloorSeconds(aReadPrepareParams.mMinIntervalFloorSeconds)
674-
.MaxIntervalCeilingSeconds(aReadPrepareParams.mMaxIntervalCeilingSeconds)
675-
.KeepSubscriptions(aReadPrepareParams.mKeepSubscriptions)
676-
.IsFabricFiltered(false)
677-
.EndOfSubscribeRequestMessage();
677+
request.IsFabricFiltered(false).EndOfSubscribeRequestMessage();
678678
SuccessOrExit(err = request.GetError());
679679

680680
err = writer.Finalize(&msgBuf);

‎src/app/WriteClient.cpp

+5-6
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,10 @@ CHIP_ERROR WriteClient::Init(Messaging::ExchangeManager * apExchangeMgr, Callbac
4343
mMessageWriter.Init(std::move(packet));
4444

4545
ReturnErrorOnFailure(mWriteRequestBuilder.Init(&mMessageWriter));
46-
mWriteRequestBuilder.TimedRequest(false).IsFabricFiltered(false);
46+
mWriteRequestBuilder.TimedRequest(false);
4747
ReturnErrorOnFailure(mWriteRequestBuilder.GetError());
4848
attributeDataIBsBuilder = mWriteRequestBuilder.CreateWriteRequests();
4949
ReturnErrorOnFailure(attributeDataIBsBuilder.GetError());
50-
5150
ClearExistingExchangeContext();
5251
mpExchangeMgr = apExchangeMgr;
5352
mpCallback = apCallback;
@@ -139,6 +138,9 @@ CHIP_ERROR WriteClient::PrepareAttribute(const AttributePathParams & attributePa
139138
VerifyOrReturnError(attributePathParams.IsValidAttributePath(), CHIP_ERROR_INVALID_PATH_LIST);
140139
AttributeDataIB::Builder attributeDataIB = mWriteRequestBuilder.GetWriteRequests().CreateAttributeDataIBBuilder();
141140
ReturnErrorOnFailure(attributeDataIB.GetError());
141+
// TODO: Add attribute version support
142+
attributeDataIB.DataVersion(0);
143+
ReturnErrorOnFailure(attributeDataIB.GetError());
142144
ReturnErrorOnFailure(attributeDataIB.CreatePath().Encode(attributePathParams));
143145
return CHIP_NO_ERROR;
144146
}
@@ -148,9 +150,6 @@ CHIP_ERROR WriteClient::FinishAttribute()
148150
CHIP_ERROR err = CHIP_NO_ERROR;
149151

150152
AttributeDataIB::Builder AttributeDataIB = mWriteRequestBuilder.GetWriteRequests().GetAttributeDataIBBuilder();
151-
152-
// TODO: Add attribute version support
153-
AttributeDataIB.DataVersion(0);
154153
AttributeDataIB.EndOfAttributeDataIB();
155154
SuccessOrExit(err = AttributeDataIB.GetError());
156155
MoveToState(State::AddAttribute);
@@ -173,7 +172,7 @@ CHIP_ERROR WriteClient::FinalizeMessage(System::PacketBufferHandle & aPacket)
173172
err = AttributeDataIBsBuilder.GetError();
174173
SuccessOrExit(err);
175174

176-
mWriteRequestBuilder.EndOfWriteRequestMessage();
175+
mWriteRequestBuilder.IsFabricFiltered(false).EndOfWriteRequestMessage();
177176
err = mWriteRequestBuilder.GetError();
178177
SuccessOrExit(err);
179178

‎src/app/tests/TestMessageDef.cpp

+17-16
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ void BuildEventDataIB(nlTestSuite * apSuite, EventDataIB::Builder & aEventDataIB
323323
NL_TEST_ASSERT(apSuite, eventPathBuilder.GetError() == CHIP_NO_ERROR);
324324
BuildEventPath(apSuite, eventPathBuilder);
325325

326-
aEventDataIBBuilder.Priority(2).EventNumber(3).EpochTimestamp(4).SystemTimestamp(5).DeltaEpochTimestamp(6).DeltaSystemTimestamp(
326+
aEventDataIBBuilder.EventNumber(2).Priority(3).EpochTimestamp(4).SystemTimestamp(5).DeltaEpochTimestamp(6).DeltaSystemTimestamp(
327327
7);
328328
err = aEventDataIBBuilder.GetError();
329329
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
@@ -365,10 +365,10 @@ void ParseEventDataIB(nlTestSuite * apSuite, EventDataIB::Parser & aEventDataIBP
365365
err = aEventDataIBParser.GetPath(&eventPath);
366366
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
367367
}
368-
err = aEventDataIBParser.GetPriority(&priorityLevel);
369-
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR && priorityLevel == 2);
370368
err = aEventDataIBParser.GetEventNumber(&number);
371-
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR && number == 3);
369+
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR && number == 2);
370+
err = aEventDataIBParser.GetPriority(&priorityLevel);
371+
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR && priorityLevel == 3);
372372
err = aEventDataIBParser.GetEpochTimestamp(&EpochTimestamp);
373373
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR && EpochTimestamp == 4);
374374
err = aEventDataIBParser.GetSystemTimestamp(&systemTimestamp);
@@ -535,6 +535,7 @@ void BuildAttributeDataIB(nlTestSuite * apSuite, AttributeDataIB::Builder & aAtt
535535
{
536536
CHIP_ERROR err = CHIP_NO_ERROR;
537537

538+
aAttributeDataIBBuilder.DataVersion(2);
538539
AttributePathIB::Builder attributePathBuilder = aAttributeDataIBBuilder.CreatePath();
539540
NL_TEST_ASSERT(apSuite, aAttributeDataIBBuilder.GetError() == CHIP_NO_ERROR);
540541
BuildAttributePathIB(apSuite, attributePathBuilder);
@@ -553,7 +554,7 @@ void BuildAttributeDataIB(nlTestSuite * apSuite, AttributeDataIB::Builder & aAtt
553554
err = pWriter->EndContainer(dummyType);
554555
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
555556
}
556-
aAttributeDataIBBuilder.DataVersion(2);
557+
557558
err = aAttributeDataIBBuilder.GetError();
558559
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
559560
aAttributeDataIBBuilder.EndOfAttributeDataIB();
@@ -963,7 +964,7 @@ void BuildReportDataMessage(nlTestSuite * apSuite, chip::TLV::TLVWriter & aWrite
963964
err = reportDataMessageBuilder.Init(&aWriter);
964965
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
965966

966-
reportDataMessageBuilder.SuppressResponse(true).SubscriptionId(2);
967+
reportDataMessageBuilder.SubscriptionId(2);
967968
NL_TEST_ASSERT(apSuite, reportDataMessageBuilder.GetError() == CHIP_NO_ERROR);
968969

969970
AttributeReportIBs::Builder AttributeReportIBs = reportDataMessageBuilder.CreateAttributeReportIBs();
@@ -974,7 +975,7 @@ void BuildReportDataMessage(nlTestSuite * apSuite, chip::TLV::TLVWriter & aWrite
974975
NL_TEST_ASSERT(apSuite, reportDataMessageBuilder.GetError() == CHIP_NO_ERROR);
975976
BuildEventReports(apSuite, EventReportIBs);
976977

977-
reportDataMessageBuilder.MoreChunkedMessages(true);
978+
reportDataMessageBuilder.MoreChunkedMessages(true).SuppressResponse(true);
978979
NL_TEST_ASSERT(apSuite, reportDataMessageBuilder.GetError() == CHIP_NO_ERROR);
979980

980981
reportDataMessageBuilder.EndOfReportDataMessage();
@@ -1170,26 +1171,26 @@ void BuildSubscribeRequestMessage(nlTestSuite * apSuite, chip::TLV::TLVWriter &
11701171
err = subscribeRequestBuilder.Init(&aWriter);
11711172
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
11721173

1173-
AttributePathIBs::Builder attributePathIBs = subscribeRequestBuilder.CreateAttributeRequests();
1174+
subscribeRequestBuilder.KeepSubscriptions(true);
11741175
NL_TEST_ASSERT(apSuite, subscribeRequestBuilder.GetError() == CHIP_NO_ERROR);
1175-
BuildAttributePathList(apSuite, attributePathIBs);
11761176

1177-
EventPathIBs::Builder eventPathList = subscribeRequestBuilder.CreateEventRequests();
1177+
subscribeRequestBuilder.MinIntervalFloorSeconds(2);
11781178
NL_TEST_ASSERT(apSuite, subscribeRequestBuilder.GetError() == CHIP_NO_ERROR);
1179-
BuildEventPaths(apSuite, eventPathList);
11801179

1181-
EventFilterIBs::Builder eventFilters = subscribeRequestBuilder.CreateEventFilters();
1180+
subscribeRequestBuilder.MaxIntervalCeilingSeconds(3);
11821181
NL_TEST_ASSERT(apSuite, subscribeRequestBuilder.GetError() == CHIP_NO_ERROR);
1183-
BuildEventFilters(apSuite, eventFilters);
11841182

1185-
subscribeRequestBuilder.MinIntervalFloorSeconds(2);
1183+
AttributePathIBs::Builder attributePathIBs = subscribeRequestBuilder.CreateAttributeRequests();
11861184
NL_TEST_ASSERT(apSuite, subscribeRequestBuilder.GetError() == CHIP_NO_ERROR);
1185+
BuildAttributePathList(apSuite, attributePathIBs);
11871186

1188-
subscribeRequestBuilder.MaxIntervalCeilingSeconds(3);
1187+
EventPathIBs::Builder eventPathList = subscribeRequestBuilder.CreateEventRequests();
11891188
NL_TEST_ASSERT(apSuite, subscribeRequestBuilder.GetError() == CHIP_NO_ERROR);
1189+
BuildEventPaths(apSuite, eventPathList);
11901190

1191-
subscribeRequestBuilder.KeepSubscriptions(true);
1191+
EventFilterIBs::Builder eventFilters = subscribeRequestBuilder.CreateEventFilters();
11921192
NL_TEST_ASSERT(apSuite, subscribeRequestBuilder.GetError() == CHIP_NO_ERROR);
1193+
BuildEventFilters(apSuite, eventFilters);
11931194

11941195
subscribeRequestBuilder.IsProxy(true);
11951196
NL_TEST_ASSERT(apSuite, subscribeRequestBuilder.GetError() == CHIP_NO_ERROR);

0 commit comments

Comments
 (0)