diff options
| author | Julius Werner <jwerner@chromium.org> | 2021-09-10 17:14:41 -0700 |
|---|---|---|
| committer | Julius Werner <jwerner@chromium.org> | 2021-09-15 01:19:22 +0000 |
| commit | a4c0e607258ae4e5c8b6758e26721ba673c1635b (patch) | |
| tree | 0a4790d4694c8b23c2226867a0faa821fbcafef9 /src/commonlib/bsd/include | |
| parent | a472c54a6334d3080c1e9eb35d8f0ba1b8154c42 (diff) | |
commonlib/cbfs: Fix minor parser edge cases
This patch fixes a few minor CBFS parsing edge cases that could lead to
unintended behavior: the CBFS attribute parser could have run into an
infinite loop if an attribute's length was (accidentally or maliciously)
invalid. A length of 0 would have caused it to read the same attribute
over and over again without making forward progress, while a very large
length could have caused an overflow that makes it go backwards to find
the next attribute. Also, the filename was not guaranteed to be
null-terminated which could have resulted in out-of-bounds reads on a
few error messages.
Finally, clarify the validity guarantees for CBFS header fields offered
by cbfs_walk() in the comment explaining cbfs_mdata.
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: Ie569786e5bec355b522f6580f53bdd8b16a4d726
Reviewed-on: https://review.coreboot.org/c/coreboot/+/57569
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Jakub Czapiga <jacz@semihalf.com>
Diffstat (limited to 'src/commonlib/bsd/include')
| -rw-r--r-- | src/commonlib/bsd/include/commonlib/bsd/cbfs_mdata.h | 5 |
1 files changed, 4 insertions, 1 deletions
diff --git a/src/commonlib/bsd/include/commonlib/bsd/cbfs_mdata.h b/src/commonlib/bsd/include/commonlib/bsd/cbfs_mdata.h index ed5c378973..d73a68c1af 100644 --- a/src/commonlib/bsd/include/commonlib/bsd/cbfs_mdata.h +++ b/src/commonlib/bsd/include/commonlib/bsd/cbfs_mdata.h @@ -8,7 +8,10 @@ #include <stdint.h> /* - * Helper structure to allocate space for a blob of metadata on the stack. + * Helper structure to allocate space for a blob of metadata on the stack. All functions using + * a cbfs_mdata should be getting it via cbfs_walk(), and can rely on the fact that cbfs_walk() + * has already fully validated the header (range checks for `len`, `attributes_offset` and + * `offset`, and null-termination for `filename`). * NOTE: The fields in any union cbfs_mdata or any of its substructures from cbfs_serialized.h * should always remain in the same byte order as they are stored on flash (= big endian). To * avoid byte-order confusion, fields should always and only be converted to host byte order at |