dsp: pdr: Bound check pldm_entity_association_pdr_extract()
```
../src/dsp/pdr.c: In function ‘pldm_entity_association_pdr_extract’:
../src/dsp/pdr.c:1333:35: error: use of attacker-controlled value ‘(size_t)((int)*((char *)&*pdr + 10).num_children + 1) * 6’ as allocation size without upper-bounds checking [CWE-789] [-Werror=analyzer-tainted-allocation-size]
1333 | pldm_entity *l_entities = malloc(sizeof(pldm_entity) * l_num_entities);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
Fixes: 9c76679224cf ("libpldm: Migrate to subproject")
Change-Id: I96582ae2b19d22413919ae0a6a9b94e2d3d40f39
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 54abcd9..4a3952c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -117,6 +117,7 @@
4. dsp: platform: Prevent overflow of arithmetic on event_data_length
5. dsp: platform: Bounds check encode_sensor_state_pdr()
6. dsp: platform: Bounds check encode_state_effecter_pdr()
+7. dsp: pdr: Bounds check pldm_entity_association_pdr_extract()
## [0.9.1] - 2024-09-07
diff --git a/src/dsp/pdr.c b/src/dsp/pdr.c
index 82b5f89..326233c 100644
--- a/src/dsp/pdr.c
+++ b/src/dsp/pdr.c
@@ -6,6 +6,7 @@
#include <assert.h>
#include <endian.h>
+#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
@@ -1316,21 +1317,45 @@
}
const uint8_t *start = (uint8_t *)pdr;
- const uint8_t *end LIBPLDM_CC_UNUSED =
+
+ if (UINTPTR_MAX - (uintptr_t)start <
+ (sizeof(struct pldm_pdr_hdr) + le16toh(hdr->length))) {
+ return;
+ }
+
+ if (pdr_len < (sizeof(struct pldm_pdr_hdr) + le16toh(hdr->length))) {
+ return;
+ }
+
+ const uint8_t *end =
start + sizeof(struct pldm_pdr_hdr) + le16toh(hdr->length);
start += sizeof(struct pldm_pdr_hdr);
+
+ if ((uintptr_t)end - (uintptr_t)start <
+ sizeof(struct pldm_pdr_entity_association)) {
+ return;
+ }
struct pldm_pdr_entity_association *entity_association_pdr =
(struct pldm_pdr_entity_association *)start;
+
+ if (entity_association_pdr->num_children == UINT8_MAX) {
+ return;
+ }
size_t l_num_entities = entity_association_pdr->num_children + 1;
if (l_num_entities < 2) {
return;
}
- if (start + sizeof(struct pldm_pdr_entity_association) +
- sizeof(pldm_entity) * (l_num_entities - 2) !=
- end) {
+
+ if (SIZE_MAX / sizeof(pldm_entity) < l_num_entities) {
return;
}
- pldm_entity *l_entities = malloc(sizeof(pldm_entity) * l_num_entities);
+
+ if (pdr_len - sizeof(*entity_association_pdr) + 1 <
+ sizeof(pldm_entity) * l_num_entities) {
+ return;
+ }
+
+ pldm_entity *l_entities = calloc(l_num_entities, sizeof(pldm_entity));
if (!l_entities) {
return;
}
diff --git a/tests/dsp/pdr.cpp b/tests/dsp/pdr.cpp
index ef6e4e8..30ea9ea 100644
--- a/tests/dsp/pdr.cpp
+++ b/tests/dsp/pdr.cpp
@@ -1561,12 +1561,12 @@
{
std::vector<uint8_t> pdr{};
pdr.resize(sizeof(pldm_pdr_hdr) + sizeof(pldm_pdr_entity_association) +
- sizeof(pldm_entity) * 4);
+ sizeof(pldm_entity) * 5);
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
pldm_pdr_hdr* hdr = reinterpret_cast<pldm_pdr_hdr*>(pdr.data());
hdr->type = PLDM_PDR_ENTITY_ASSOCIATION;
hdr->length =
- htole16(sizeof(pldm_pdr_entity_association) + sizeof(pldm_entity) * 4);
+ htole16(sizeof(pldm_pdr_entity_association) + sizeof(pldm_entity) * 5);
pldm_pdr_entity_association* e =
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)