GNUBoot
/
coreboot-kgpe-d16
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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>
This commit is contained in:
Alex Rebert 2020-02-29 23:03:54 -05:00 committed by Patrick Georgi
parent 56a0c2579d
commit e5e24107f9
1 changed files with 17 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
payloads/libpayload/libcbfs/cbfs_core.c
View File

@ -212,9 +212,15 @@ struct cbfs_handle *cbfs_get_handle(struct cbfs_media *media, const char *name)
} }
// Move to next file. // Move to next file.
offset += ntohl(file.len) + ntohl(file.offset); uint32_t next_offset = offset + ntohl(file.len) + ntohl(file.offset);
if (offset % CBFS_ALIGNMENT) if (next_offset % CBFS_ALIGNMENT)
offset += CBFS_ALIGNMENT - (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); media->close(media);
LOG("WARNING: '%s' not found.\n", name); LOG("WARNING: '%s' not found.\n", name);
@ -309,7 +315,14 @@ void *cbfs_get_attr(struct cbfs_handle *handle, uint32_t tag)
return NULL; return NULL;
} }
if (ntohl(attr.tag) != tag) { 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; continue;
} }
ret = m->map(m, offset, ntohl(attr.len)); ret = m->map(m, offset, ntohl(attr.len));