ACK: [PATCH] uefi: uefidump: add some guarding on loop iteration

[ivanhu]

On 11/10/2017 06:23 PM, Colin King wrote:
> From: Colin Ian King <colin.king at canonical.com>
>
> Add some sanity checking to size boundaries. Cleans up a static
> analysis warning.
>
> Detected by CoverityScan, CID#1338583 ("Insecure data handling")
>
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
>   src/uefi/uefidump/uefidump.c | 17 ++++++++++++-----
>   1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/src/uefi/uefidump/uefidump.c b/src/uefi/uefidump/uefidump.c
> index 55ce7f23..80ea2a99 100644
> --- a/src/uefi/uefidump/uefidump.c
> +++ b/src/uefi/uefidump/uefidump.c
> @@ -554,15 +554,22 @@ static char *uefidump_build_dev_path(char *path, fwts_uefi_dev_path *dev_path, c
>   			break;
>   		case FWTS_UEFI_DNS_DEVICE_PATH_SUBTYPE:
>   			if (dev_path_len > sizeof(fwts_uefi_dns_dev_path)) {
> -				size_t i;
>   				fwts_uefi_dns_dev_path *d = (fwts_uefi_dns_dev_path *)dev_path;
>   				uint16_t len = d->dev_path.length[0] | (((uint16_t)d->dev_path.length[1]) << 8);
> +				ssize_t i, sz = (ssize_t)len - sizeof(fwts_uefi_dns_dev_path);
>   				path = uefidump_vprintf(path, "\\DNS(0x%" PRIx8 ",", d->isipv6);
>   
> -				/* dump one or more DNS server address */
> -				for (i = 0; i < (len - sizeof(fwts_uefi_dns_dev_path)); i++)
> -					path = uefidump_vprintf(path, "%02" PRIx8 , d->dns_addr[i]);
> -				path = uefidump_vprintf(path, ")");
> +				/*
> +				 *  Add a little more sanity checking, but we can never be sure
> +				 *  we will not overrun d->dns_addr as we have to trust the given
> +				 *  length.
> +				 */
> +				if ((sz > 0) && (sz <= 0xffff)) {
> +					/* dump one or more DNS server address */
> +					for (i = 0; i < sz; i++)
> +						path = uefidump_vprintf(path, "%02" PRIx8 , d->dns_addr[i]);
> +					path = uefidump_vprintf(path, ")");
> +				}
>   			}
>   			break;
>   		default:
Acked-by: Ivan Hu <ivan.hu at canonical.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/fwts-devel/attachments/20171114/7d41b5e3/attachment.html>