ACK: [PATCH] uefidump: add more bounds checking and remove need for heap

[Alex Hung]

On 2015-11-24 09:06 PM, Colin King wrote:
> From: Colin Ian King <colin.king at canonical.com>
>
> CoverityScan found a buffer overflow where the provided size of the
> string may be actually larger than the buffer itself. Add some more
> sanity checking on the buffer.  Also, don't allocate a tmp buffer
> from the heap, use the stack instead.
>
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
>   src/uefi/uefidump/uefidump.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/src/uefi/uefidump/uefidump.c b/src/uefi/uefidump/uefidump.c
> index 8bb9f00..dc4fa0f 100644
> --- a/src/uefi/uefidump/uefidump.c
> +++ b/src/uefi/uefidump/uefidump.c
> @@ -487,14 +487,15 @@ static char *uefidump_build_dev_path(char *path, fwts_uefi_dev_path *dev_path, c
>   		case FWTS_UEFI_URI_DEVICE_PATH_SUBTYPE:
>   			if (dev_path_len >= sizeof(fwts_uefi_uri_dev_path)) {
>   				fwts_uefi_uri_dev_path *u = (fwts_uefi_uri_dev_path *)dev_path;
> -				char *tmp;
>   				uint16_t len = u->dev_path.length[0] | (((uint16_t)u->dev_path.length[1]) << 8);
> -				tmp = malloc(len - sizeof(fwts_uefi_uri_dev_path) + 1);
> -				if (tmp) {
> -					memcpy(tmp, u->uri, len - sizeof(fwts_uefi_uri_dev_path));
> -					tmp[len - sizeof(fwts_uefi_uri_dev_path)] = '\0';
> +				if ((len > sizeof(fwts_uefi_uri_dev_path)) &&
> +				    (len <= dev_path_len - sizeof(fwts_uefi_uri_dev_path))) {
> +					uint16_t tmp_len = len - sizeof(fwts_uefi_uri_dev_path);
> +					char tmp[tmp_len + 1];
> +
> +					memcpy(tmp, u->uri, tmp_len);
> +					tmp[tmp_len] = '\0';
>   					path = uefidump_vprintf(path, "\\URI(%s)", tmp);
> -					free(tmp);
>   				}
>   			}
>   			break;
>

Acked-by: Alex Hung <alex.hung at canonical.com>