From e9ce922ddf2ec6c1095f42ee9857f369084761c3 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Tue, 9 May 2017 15:34:08 -0400 Subject: [PATCH 23/24] efi_loadopt_create(): avoid NULL dereference covscan rightly points out that dp is allowed to be NULL (and so is buf), so we can't pass those in to memcpy() in those cases. So don't. Signed-off-by: Peter Jones --- src/loadopt.c | 46 +++++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/src/loadopt.c b/src/loadopt.c index 5301f3d..cf0886d 100644 --- a/src/loadopt.c +++ b/src/loadopt.c @@ -56,36 +56,44 @@ efi_loadopt_create(uint8_t *buf, ssize_t size, uint32_t attributes, } if (!buf) { +invalid: errno = EINVAL; return -1; } - if (!optional_data && optional_data_size != 0) { - errno = EINVAL; - return -1; - } + if (!optional_data && optional_data_size != 0) + goto invalid; - if (!dp && dp_size == 0) { - errno = EINVAL; - return -1; - } + if ((!dp && dp_size == 0) || dp_size < 0) + goto invalid; + + if (dp) { + if (!efidp_is_valid(dp, dp_size)) + goto invalid; - uint8_t *pos = buf; + if (efidp_size(dp) != dp_size) + goto invalid; + } - *(uint32_t *)pos = attributes; - pos += sizeof (attributes); + if (buf) { + uint8_t *pos = buf; + *(uint32_t *)pos = attributes; + pos += sizeof (attributes); - *(uint16_t *)pos = dp_size; - pos += sizeof (uint16_t); + *(uint16_t *)pos = dp_size; + pos += sizeof (uint16_t); - utf8_to_ucs2((uint16_t *)pos, desc_len, 1, (uint8_t *)description); - pos += desc_len; + utf8_to_ucs2((uint16_t *)pos, desc_len, 1, + (uint8_t *)description); + pos += desc_len; - memcpy(pos, dp, dp_size); - pos += dp_size; + if (dp) + memcpy(pos, dp, dp_size); + pos += dp_size; - if (optional_data && optional_data_size > 0) - memcpy(pos, optional_data, optional_data_size); + if (optional_data && optional_data_size > 0) + memcpy(pos, optional_data, optional_data_size); + } return sz; } -- 2.12.2