@@ -162,11 +162,13 @@ static int get_esl_cert(const char *buf, const size_t buflen, char **cert)
/*
* Extracts size of the PKCS7 signed data embedded in the
* struct Authentication 2 Descriptor Header.
+ *
+ * Returns a ssize_t: positive values are OK, negative values are error.
*/
-static size_t get_pkcs7_len(const struct efi_variable_authentication_2 *auth)
+static ssize_t get_pkcs7_len(const struct efi_variable_authentication_2 *auth)
{
uint32_t dw_length;
- size_t size;
+ ssize_t size;
assert(auth != NULL);
@@ -176,14 +178,17 @@ static size_t get_pkcs7_len(const struct efi_variable_authentication_2 *auth)
+ sizeof(auth->auth_info.hdr.w_certificate_type)
+ sizeof(auth->auth_info.cert_type));
+ if (size < 0)
+ return OPAL_PARAMETER;
+
return size;
}
int get_auth_descriptor2(const void *buf, const size_t buflen, void **auth_buffer)
{
const struct efi_variable_authentication_2 *auth = buf;
- int auth_buffer_size;
- size_t len;
+ ssize_t auth_buffer_size;
+ ssize_t len;
assert(auth_buffer != NULL);
if (buflen < sizeof(struct efi_variable_authentication_2)
@@ -192,7 +197,7 @@ int get_auth_descriptor2(const void *buf, const size_t buflen, void **auth_buffe
len = get_pkcs7_len(auth);
/* pkcs7 content length cannot be greater than buflen */
- if (len > buflen)
+ if (len < 0 || len > buflen)
return OPAL_PARAMETER;
auth_buffer_size = sizeof(auth->timestamp) + sizeof(auth->auth_info.hdr)
@@ -417,11 +422,13 @@ int check_timestamp(const char *key, const struct efi_time *timestamp,
static mbedtls_pkcs7* get_pkcs7(const struct efi_variable_authentication_2 *auth)
{
char *checkpkcs7cert = NULL;
- size_t len;
+ ssize_t len;
mbedtls_pkcs7 *pkcs7 = NULL;
int rc;
len = get_pkcs7_len(auth);
+ if (len < 0)
+ return NULL;
pkcs7 = malloc(sizeof(struct mbedtls_pkcs7));
if (!pkcs7)
get_pkcs7_len parses a efi_variable_authentication2 and tells us how long the embedded PKCS#7 message is. This process involves reading a number out of the auth header and subtracting a number of constants. As such, this process can fail: if the resultant value is less than 0, this is an error: a pkcs7 message cannot have negative size! Currently, we don't catch this: we calculate with an unsigned type, so it experiences integer overflow and becomes enormous. The field we're reading from the header is 32 bit, so we don't need all our 64 bits as data bits. Instead, change the type to be ssize_t, and if the result of the subtraction is less than 0, return OPAL_PARAMETER. Adjust call-sites to use the different return type and specifically catch return values that are errors. Signed-off-by: Daniel Axtens <dja@axtens.net> --- I am fairly sure this cannot cause a bug. If you could get to get_pkcs7 then you'd get to mbedtls_pkcs7_parse_der with a huge len, but the only caller is verify_signature(), which is only called by process_update(). However process_update() first calls get_auth_descriptor2(), which will helpfully call get_pkcs7_len() and compare the result with buf_len. The underflowed negative number will be bigger than buf_len - so the caller (get_auth_descriptor2()) will fail, and process_update() will then bail before calling into verify_signature(). So this patch isn't as urgent as the others. --- libstb/secvar/backend/edk2-compat-process.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)