summaryrefslogtreecommitdiff
path: root/payloads/libpayload
diff options
context:
space:
mode:
authorAlex Rebert <alexandre.rebert@gmail.com>2020-02-29 23:03:54 -0500
committerPatrick Georgi <pgeorgi@google.com>2020-03-02 15:00:24 +0000
commite5e24107f91a959e24546d0cdad84eecee329f8e (patch)
treeefc80350a92f09d1e8cea8fcf76d23e5cefdb106 /payloads/libpayload
parent56a0c2579dd5a779ef01b2dbd736520c0271381a (diff)
downloadcoreboot-e5e24107f91a959e24546d0cdad84eecee329f8e.tar.xz
libpayload: cbfs: fix infinite loop in cbfs_get_{handle,attr}
cbfs_get_handle() and cbfs_get_attr() are both looping over elements to find a particular one. Each element header contains the element's length, which is used to compute the next element's offset. Invalid or corrupted CBFS files could lead to infinite loops where the offset would remain constant across iterations, due to 0-length elements or integer overflows in the computation of the next offset. This patch makes both functions more robust by adding a check that ensure offsets are strictly monotonic. Instead of infinite looping, the functions are now printing an ERROR and returning a NULL value. Change-Id: I440e82fa969b8c2aacc5800e7e26450c3b97c74a Signed-off-by: Alex Rebert <alexandre.rebert@gmail.com> Found-by: Mayhem Reviewed-on: https://review.coreboot.org/c/coreboot/+/39177 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Diffstat (limited to 'payloads/libpayload')
-rw-r--r--payloads/libpayload/libcbfs/cbfs_core.c21
1 files changed, 17 insertions, 4 deletions
diff --git a/payloads/libpayload/libcbfs/cbfs_core.c b/payloads/libpayload/libcbfs/cbfs_core.c
index e94e1e76ba..30a41f8a72 100644
--- a/payloads/libpayload/libcbfs/cbfs_core.c
+++ b/payloads/libpayload/libcbfs/cbfs_core.c
@@ -212,9 +212,15 @@ struct cbfs_handle *cbfs_get_handle(struct cbfs_media *media, const char *name)
}
// Move to next file.
- offset += ntohl(file.len) + ntohl(file.offset);
- if (offset % CBFS_ALIGNMENT)
- offset += CBFS_ALIGNMENT - (offset % CBFS_ALIGNMENT);
+ uint32_t next_offset = offset + ntohl(file.len) + ntohl(file.offset);
+ if (next_offset % CBFS_ALIGNMENT)
+ next_offset += CBFS_ALIGNMENT - (next_offset % CBFS_ALIGNMENT);
+ // Check that offset is strictly monotonic to prevent infinite loop
+ if (next_offset <= offset) {
+ ERROR("ERROR: corrupted CBFS file header at 0x%x.\n", offset);
+ break;
+ }
+ offset = next_offset;
}
media->close(media);
LOG("WARNING: '%s' not found.\n", name);
@@ -309,7 +315,14 @@ void *cbfs_get_attr(struct cbfs_handle *handle, uint32_t tag)
return NULL;
}
if (ntohl(attr.tag) != tag) {
- offset += ntohl(attr.len);
+ uint32_t next_offset = offset + ntohl(attr.len);
+ // Check that offset is strictly monotonic to prevent infinite loop
+ if (next_offset <= offset) {
+ ERROR("ERROR: corrupted CBFS attribute at 0x%x.\n", offset);
+ m->close(m);
+ return NULL;
+ }
+ offset = next_offset;
continue;
}
ret = m->map(m, offset, ntohl(attr.len));