Skip to content

Commit

Permalink
Fix attribute reads into a large buffer to not read random memory.
Browse files Browse the repository at this point in the history
When reading an attribute value into a large buffer, we would read the
number of bytes that can fit in the buffer, not the number of bytes
the attribute value actually takes up.  This would cause us to read
whatever memory came after the attribute value.

Also fixes some issues in typeSensitiveMemCopy when trying to
read a string-valued attribute into a buffer that's not even big
enough to fit the length value of the string.

Fixes #4371
  • Loading branch information
bzbarsky-apple committed Jan 15, 2021
1 parent be3279a commit 041988c
Showing 1 changed file with 19 additions and 6 deletions.
25 changes: 19 additions & 6 deletions src/app/util/attribute-storage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -365,33 +365,46 @@ static uint8_t * singletonAttributeLocation(EmberAfAttributeMetadata * am)
// This function does mem copy, but smartly, which means that if the type is a
// string, it will copy as much as it can.
// If src == NULL, then this method will set memory to zeroes
// See documentation for emAfReadOrWriteAttribute for the semantics of
// readLength when reading and writing.
static EmberAfStatus typeSensitiveMemCopy(uint8_t * dest, uint8_t * src, EmberAfAttributeMetadata * am, bool write,
uint16_t readLength)
{
EmberAfAttributeType attributeType = am->attributeType;
uint16_t size = (readLength == 0) ? am->size : readLength;
// readLength == 0 for a read indicates that we should just trust that the
// caller has enough space for an attribute...
bool ignoreReadLength = write || (readLength == 0);
uint16_t bufferSize = ignoreReadLength ? am->size : readLength;

if (emberAfIsStringAttributeType(attributeType))
{
emberAfCopyString(dest, src, static_cast<uint8_t>(size - 1));
if (bufferSize < 1)
{
return EMBER_ZCL_STATUS_INSUFFICIENT_SPACE;
}
emberAfCopyString(dest, src, static_cast<uint8_t>(bufferSize - 1));
}
else if (emberAfIsLongStringAttributeType(attributeType))
{
emberAfCopyLongString(dest, src, static_cast<uint16_t>(size - 2));
if (bufferSize < 2)
{
return EMBER_ZCL_STATUS_INSUFFICIENT_SPACE;
}
emberAfCopyLongString(dest, src, static_cast<uint16_t>(bufferSize - 2));
}
else
{
if (!write && readLength != 0 && readLength < am->size)
if (!ignoreReadLength && readLength < am->size)
{
return EMBER_ZCL_STATUS_INSUFFICIENT_SPACE;
}
if (src == NULL)
{
memset(dest, 0, size);
memset(dest, 0, am->size);
}
else
{
memmove(dest, src, size);
memmove(dest, src, am->size);
}
}
return EMBER_ZCL_STATUS_SUCCESS;
Expand Down

0 comments on commit 041988c

Please sign in to comment.