Skip to content

Commit 0583626

Browse files
Prevent Buffer Overflows with External Attribute Reads
* Prevent buffer overflows by blocking creation of dynamic endpoints with attributes that would overflow the static attribute buffer * Added additional sanity checks to block reads of attributes that would overflow the attribute reads; return RESOURCE_EXHAUSTED instead
1 parent 6880437 commit 0583626

File tree

1 file changed

+34
-4
lines changed

1 file changed

+34
-4
lines changed

src/app/util/attribute-storage.cpp

+34-4
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include <lib/support/logging/CHIPLogging.h>
3434
#include <platform/LockTracker.h>
3535
#include <protocols/interaction_model/StatusCode.h>
36+
#include <app/util/ember-io-storage.h>
3637

3738
using chip::Protocols::InteractionModel::Status;
3839

@@ -291,6 +292,27 @@ CHIP_ERROR emberAfSetDynamicEndpoint(uint16_t index, EndpointId id, const EmberA
291292
}
292293
}
293294

295+
for (uint8_t i = 0; ep && (i < ep->clusterCount); i++) {
296+
if (!ep->cluster) {
297+
continue;
298+
}
299+
300+
const EmberAfCluster * cluster = &(ep->cluster[i]);
301+
if (!cluster->attributes) {
302+
continue;
303+
}
304+
305+
for (uint16_t j = 0; j < cluster->attributeCount; j++) {
306+
const EmberAfAttributeMetadata * attr = &(cluster->attributes[j]);
307+
if (emberAfAttributeSize(attr) > gEmberAttributeIOBufferSpan.size()) {
308+
ChipLogError(
309+
DataManagement,
310+
"Attribute %u (id=" ChipLogFormatMEI ") of Cluster %u (id=" ChipLogFormatMEI ") too large",
311+
j, ChipLogValueMEI(attr->attributeId), i, ChipLogValueMEI(cluster->clusterId));
312+
return CHIP_ERROR_NO_MEMORY;
313+
}
314+
}
315+
}
294316
emAfEndpoints[index].endpoint = id;
295317
emAfEndpoints[index].deviceTypeList = deviceTypeList;
296318
emAfEndpoints[index].endpointType = ep;
@@ -639,10 +661,18 @@ Status emAfReadOrWriteAttribute(const EmberAfAttributeSearchRecord * attRecord,
639661
// Is the attribute externally stored?
640662
if (am->mask & ATTRIBUTE_MASK_EXTERNAL_STORAGE)
641663
{
642-
return (write ? emberAfExternalAttributeWriteCallback(attRecord->endpoint, attRecord->clusterId,
643-
am, buffer)
644-
: emberAfExternalAttributeReadCallback(attRecord->endpoint, attRecord->clusterId,
645-
am, buffer, emberAfAttributeSize(am)));
664+
if (write) {
665+
return emberAfExternalAttributeWriteCallback(attRecord->endpoint, attRecord->clusterId,
666+
am, buffer);
667+
}
668+
669+
if (readLength < emberAfAttributeSize(am)) {
670+
// Prevent a potential buffer overflow
671+
return Status::ResourceExhausted;
672+
}
673+
674+
return emberAfExternalAttributeReadCallback(attRecord->endpoint, attRecord->clusterId,
675+
am, buffer, emberAfAttributeSize(am));
646676
}
647677

648678
// Internal storage is only supported for fixed endpoints

0 commit comments

Comments
 (0)