-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix too large buffer size in VerifyStatusReport() #4765
Conversation
Can we do something like Reader reader(msg) |
BufferReader doesn't have |
I strongly feel like adding |
I'm happy to add this as part of this PR |
Yes, please. |
@@ -129,8 +129,13 @@ void VerifyStatusReport(nlTestSuite * inSuite, void * inContext, const System::P | |||
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); | |||
NL_TEST_ASSERT(inSuite, payloadHeader.GetProtocolID() == Protocols::kProtocol_Protocol_Common); | |||
NL_TEST_ASSERT(inSuite, payloadHeader.GetMessageType() == static_cast<uint8_t>(Protocols::Common::MsgType::StatusReport)); | |||
if (headerSize > msg->DataLength()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory, this should have caused err
to be an error.... I'm OK with keeping it, or ok with a comment saying we can assume headerSize
is small enough because err == CHIP_NO_ERROR
here.
Certainly non-test code makes that assumption.
|
||
Encoding::LittleEndian::Reader reader(msg->Start() + headerSize, msg->DataLength()); | ||
Encoding::LittleEndian::Reader reader(msg->Start() + headerSize, static_cast<uint16_t>(msg->DataLength() - headerSize)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably OK if we need to not change the state of msg
. Otherwise we could msg->Consume(headerSize)
or something and then ditch the arithmetic.
Problem
In
TestBdxTransferSession.cpp
,VerifyStatusReport()
, aReader
object was being initialized with a length that exceeded the bounds of the PacketBuffer to read.Summary of Changes
Subtract
headerSize
from the length parameter so that the Reader cannot read data beyond the bounds of the PacketBufferFixes #4748