Patrick Georgi submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved
lz4: Fix out-of-bounds reads

Fix two out-of-bounds reads in lz4 decompression:

1) LZ4_decompress_generic could read one byte past the input buffer when
decoding variable length literals due to a missing bounds check. This
issue was resolved in libpayload, commonlib and cbfstool

2) ulz4fn could read up to 4 bytes past the input buffer when reading a
lz4_block_header due to a missing bounds check. This issue was resolved
in libpayload and commonlib.

Change-Id: I5afdf7e1d43ecdb06c7b288be46813c1017569fc
Signed-off-by: Alex Rebert <alexandre.rebert@gmail.com>
Found-by: Mayhem
Reviewed-on: https://review.coreboot.org/c/coreboot/+/39174
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
---
M payloads/libpayload/liblz4/lz4.c.inc
M payloads/libpayload/liblz4/lz4_wrapper.c
M src/commonlib/bsd/lz4.c.inc
M src/commonlib/bsd/lz4_wrapper.c
M util/cbfstool/lz4/lib/lz4.c
5 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/payloads/libpayload/liblz4/lz4.c.inc b/payloads/libpayload/liblz4/lz4.c.inc
index baa9110..68fac47 100644
--- a/payloads/libpayload/liblz4/lz4.c.inc
+++ b/payloads/libpayload/liblz4/lz4.c.inc
@@ -150,6 +150,7 @@
if ((length=(token>>ML_BITS)) == RUN_MASK)
{
unsigned s;
+ if ((endOnInput) && unlikely(ip>=iend-RUN_MASK)) goto _output_error; /* overflow detection */
do
{
s = *ip++;
diff --git a/payloads/libpayload/liblz4/lz4_wrapper.c b/payloads/libpayload/liblz4/lz4_wrapper.c
index d125ce33..3d17fe6 100644
--- a/payloads/libpayload/liblz4/lz4_wrapper.c
+++ b/payloads/libpayload/liblz4/lz4_wrapper.c
@@ -141,6 +141,9 @@
}

while (1) {
+ if ((size_t)(in - src) + sizeof(struct lz4_block_header) > srcn)
+ break; /* input overrun */
+
struct lz4_block_header b = { .raw = le32toh(*(uint32_t *)in) };
in += sizeof(struct lz4_block_header);

diff --git a/src/commonlib/bsd/lz4.c.inc b/src/commonlib/bsd/lz4.c.inc
index b3be4e5..8c75e2f 100644
--- a/src/commonlib/bsd/lz4.c.inc
+++ b/src/commonlib/bsd/lz4.c.inc
@@ -150,6 +150,7 @@
if ((length=(token>>ML_BITS)) == RUN_MASK)
{
unsigned s;
+ if ((endOnInput) && unlikely(ip>=iend-RUN_MASK)) goto _output_error; /* overflow detection */
do
{
s = *ip++;
diff --git a/src/commonlib/bsd/lz4_wrapper.c b/src/commonlib/bsd/lz4_wrapper.c
index 2367afc..3822e8c 100644
--- a/src/commonlib/bsd/lz4_wrapper.c
+++ b/src/commonlib/bsd/lz4_wrapper.c
@@ -129,6 +129,9 @@
}

while (1) {
+ if ((size_t)(in - src) + sizeof(struct lz4_block_header) > srcn)
+ break; /* input overrun */
+
struct lz4_block_header b = {
{ .raw = le32toh(*(const uint32_t *)in) }
};
diff --git a/util/cbfstool/lz4/lib/lz4.c b/util/cbfstool/lz4/lib/lz4.c
index 9c9a9a0..e393690 100644
--- a/util/cbfstool/lz4/lib/lz4.c
+++ b/util/cbfstool/lz4/lib/lz4.c
@@ -1206,6 +1206,7 @@
if ((length=(token>>ML_BITS)) == RUN_MASK)
{
unsigned s;
+ if ((endOnInput) && unlikely(ip>=iend-RUN_MASK)) goto _output_error; /* overflow detection */
do
{
s = *ip++;

To view, visit change 39174. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5afdf7e1d43ecdb06c7b288be46813c1017569fc
Gerrit-Change-Number: 39174
Gerrit-PatchSet: 2
Gerrit-Owner: Alexandre Rebert <alexandre.rebert@gmail.com>
Gerrit-Reviewer: Alexandre Rebert <alexandre.rebert@gmail.com>
Gerrit-Reviewer: Julius Werner <jwerner@chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth@google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-MessageType: merged