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

Colin King colin.king at canonical.com
Fri Nov 10 10:23:03 UTC 2017


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:
-- 
2.14.1




More information about the fwts-devel mailing list