From f6317b4892d8d295312ffd7a8b4caccf2610c25e Mon Sep 17 00:00:00 2001 From: Hung-Te Lin Date: Wed, 12 Jun 2019 16:59:24 +0800 Subject: [PATCH] src/driver/vpd: Update vpd_decode from upstream The upstream vpd_decode.c has been revised to prevent overrun of decoded contents. BUG=chromium:967209 TEST=select VPD config on kukui; make; boots on at least kukui boards. Change-Id: I1a50670a66b7b174d2a432c29d90152b86c32982 Signed-off-by: Hung-Te Lin Reviewed-on: https://review.coreboot.org/c/coreboot/+/33414 Tested-by: build bot (Jenkins) Reviewed-by: Julius Werner --- src/drivers/vpd/vpd_decode.c | 51 ++++++++++++++++++++---------------- src/drivers/vpd/vpd_decode.h | 21 ++------------- 2 files changed, 30 insertions(+), 42 deletions(-) diff --git a/src/drivers/vpd/vpd_decode.c b/src/drivers/vpd/vpd_decode.c index 0eab704ca8..527c50818e 100644 --- a/src/drivers/vpd/vpd_decode.c +++ b/src/drivers/vpd/vpd_decode.c @@ -8,7 +8,7 @@ */ #include "vpd_decode.h" -int vpd_decode_len( +static int vpd_decode_len( const u32 max_len, const u8 *in, u32 *length, u32 *decoded_len) { u8 more; @@ -32,15 +32,36 @@ int vpd_decode_len( return VPD_DECODE_OK; } +static int vpd_decode_entry( + const u32 max_len, const u8 *input_buf, u32 *consumed, + const u8 **entry, u32 *entry_len) +{ + u32 decoded_len; + + if (vpd_decode_len(max_len - *consumed, &input_buf[*consumed], + entry_len, &decoded_len) != VPD_DECODE_OK) + return VPD_DECODE_FAIL; + if (max_len - *consumed < decoded_len) + return VPD_DECODE_FAIL; + + *consumed += decoded_len; + *entry = input_buf + *consumed; + + /* entry_len is untrusted data and must be checked again. */ + if (max_len - *consumed < *entry_len) + return VPD_DECODE_FAIL; + + *consumed += *entry_len; + return VPD_DECODE_OK; +} + int vpd_decode_string( const u32 max_len, const u8 *input_buf, u32 *consumed, vpd_decode_callback callback, void *callback_arg) { int type; - int res; u32 key_len; u32 value_len; - u32 decoded_len; const u8 *key; const u8 *value; @@ -55,30 +76,14 @@ int vpd_decode_string( case VPD_TYPE_STRING: (*consumed)++; - /* key */ - res = vpd_decode_len(max_len - *consumed, &input_buf[*consumed], - &key_len, &decoded_len); - /* key name cannot be empty, and must be followed by value. */ - if (res != VPD_DECODE_OK || key_len < 1 || - *consumed + decoded_len + key_len >= max_len) + if (vpd_decode_entry(max_len, input_buf, consumed, &key, + &key_len) != VPD_DECODE_OK) return VPD_DECODE_FAIL; - *consumed += decoded_len; - key = &input_buf[*consumed]; - *consumed += key_len; - - /* value */ - res = vpd_decode_len(max_len - *consumed, &input_buf[*consumed], - &value_len, &decoded_len); - /* value can be empty (value_len = 0). */ - if (res != VPD_DECODE_OK || - *consumed + decoded_len + value_len > max_len) + if (vpd_decode_entry(max_len, input_buf, consumed, &value, + &value_len) != VPD_DECODE_OK) return VPD_DECODE_FAIL; - *consumed += decoded_len; - value = &input_buf[*consumed]; - *consumed += value_len; - if (type == VPD_TYPE_STRING) return callback(key, key_len, value, value_len, callback_arg); diff --git a/src/drivers/vpd/vpd_decode.h b/src/drivers/vpd/vpd_decode.h index 99ca7efa81..5d595f367f 100644 --- a/src/drivers/vpd/vpd_decode.h +++ b/src/drivers/vpd/vpd_decode.h @@ -29,29 +29,12 @@ typedef int vpd_decode_callback( const u8 *key, u32 key_len, const u8 *value, u32 value_len, void *arg); -/* - * vpd_decode_len - * - * Given an encoded string, this function extracts the length of content - * (either key or value). The *consumed will contain the number of bytes - * consumed. - * - * The input_buf points to the first byte of the input buffer. - * - * The *consumed starts from 0, which is actually the next byte to be decoded. - * It can be non-zero to be used in multiple calls. - * - * Returns VPD_DECODE_OK on success, otherwise VPD_DECODE_FAIL. - */ -int vpd_decode_len( - const u32 max_len, const u8 *in, u32 *length, u32 *decoded_len); - /* * vpd_decode_string * * Given the encoded string, this function invokes callback with extracted - * (key, value). The *consumed will be plused the number of bytes consumed in - * this function. + * (key, value). The *consumed will be incremented by the number of bytes + * consumed in this function. * * The input_buf points to the first byte of the input buffer. *