boot: make device_path_next_node() robust against malformed zero-length nodes

device_path_next_node() advances by the current node's Length field,
which per the EFI device path protocol includes the 4-byte node header;
a well-formed node is therefore at least sizeof(EFI_DEVICE_PATH) bytes
long. A malformed node with Length < sizeof(EFI_DEVICE_PATH), in
particular Length == 0, makes the helper return its input pointer
unchanged.

Advance by at least sizeof(EFI_DEVICE_PATH).

Follow-up for 5080a60a719da213fa90964b76cc90bd0d1cb8de

diff --git a/src/boot/device-path-util.c b/src/boot/device-path-util.c
index 5243e81e5df18b33bbd6c29c45f642f0edfeb083..73c3933299c07798600862119a9581171fedc0f9 100644 (file)
--- a/src/boot/device-path-util.c
+++ b/src/boot/device-path-util.c
@@ -25,6 +25,10 @@ EFI_STATUS make_file_device_path(EFI_HANDLE device, const char16_t *file, EFI_DE
         const EFI_DEVICE_PATH *end_node = device_path_find_end_node(dp);
 
         size_t file_size = strsize16(file);
+        /* The node Length is a uint16_t, so refuse a path that would not fit. */
+        if (file_size > UINT16_MAX - sizeof(FILEPATH_DEVICE_PATH))
+                return EFI_INVALID_PARAMETER;
+
         size_t dp_size = (uint8_t *) end_node - (uint8_t *) dp;
 
         /* Make a copy that can also hold a file media device path. */
@@ -55,6 +59,9 @@ EFI_STATUS make_url_device_path(const char16_t *url, EFI_DEVICE_PATH **ret) {
                 return EFI_INVALID_PARAMETER;
 
         size_t l = strlen8(u);
+        /* The node Length is a uint16_t, so refuse a URL that would not fit. */
+        if (l > UINT16_MAX - offsetof(URI_DEVICE_PATH, Uri))
+                return EFI_INVALID_PARAMETER;
 
         size_t t = offsetof(URI_DEVICE_PATH, Uri) + l + sizeof(EFI_DEVICE_PATH);
         EFI_DEVICE_PATH *dp = xmalloc(t);
@@ -177,6 +184,23 @@ EFI_DEVICE_PATH *device_path_replace_node(
         return ret;
 }
 
+bool device_path_is_valid(const EFI_DEVICE_PATH *dp, size_t size) {
+        if (!dp)
+                return false;
+
+        /* Validate against the known size so a truncated/corrupt path can't make us read out of bounds. */
+        for (;;) {
+                if (size < sizeof(EFI_DEVICE_PATH))
+                        return false;
+                if (dp->Length < sizeof(EFI_DEVICE_PATH) || dp->Length > size)
+                        return false;
+                if (device_path_is_end(dp))
+                        return true;
+                size -= dp->Length;
+                dp = (const EFI_DEVICE_PATH *) ((const uint8_t *) dp + dp->Length);
+        }
+}
+
 size_t device_path_size(const EFI_DEVICE_PATH *dp) {
         const EFI_DEVICE_PATH *i = ASSERT_PTR(dp);
 

diff --git a/src/boot/device-path-util.h b/src/boot/device-path-util.h
index b02cf3146a9ecaca6b7b3f255b03d8097b1e9b79..8a993f27fe909cd7b831d6e1f09b101b5a07b4fd 100644 (file)
--- a/src/boot/device-path-util.h
+++ b/src/boot/device-path-util.h
@@ -13,6 +13,9 @@ EFI_DEVICE_PATH *device_path_replace_node(
 
 static inline EFI_DEVICE_PATH *device_path_next_node(const EFI_DEVICE_PATH *dp) {
         assert(dp);
+        /* The node Length includes the 4-byte header, so a well-formed node is at least that long. Paths
+         * coming from untrusted sources must be checked with device_path_is_valid() before being walked. */
+        assert(dp->Length >= sizeof(EFI_DEVICE_PATH));
         return (EFI_DEVICE_PATH *) ((uint8_t *) dp + dp->Length);
 }
 
@@ -30,4 +33,8 @@ static inline bool device_path_is_end(const EFI_DEVICE_PATH *dp) {
 
 size_t device_path_size(const EFI_DEVICE_PATH *dp);
 
+/* Validates that a device path is well-formed and fully contained within the given size, terminated by an
+ * end node. Use on paths from untrusted sources (e.g. EFI variables) before walking them. */
+bool device_path_is_valid(const EFI_DEVICE_PATH *dp, size_t size);
+
 EFI_DEVICE_PATH *device_path_dup(const EFI_DEVICE_PATH *dp);