|
 |
e1263a |
From 50b302ea7b6bd41c38d50b2af9d89af5f715068a Mon Sep 17 00:00:00 2001
|
|
|
e1263a |
From: Laszlo Ersek <lersek@redhat.com>
|
|
|
e1263a |
Date: Wed, 16 May 2018 14:06:48 +0200
|
|
|
e1263a |
Subject: [PATCH] fix relop in esl_iter_next()
|
|
|
e1263a |
|
|
|
e1263a |
esl_iter_next() seeks to the next EFI_SIGNATURE_LIST object in the
|
|
|
e1263a |
signature database that's being processed.
|
|
|
e1263a |
|
|
|
e1263a |
- The position of the current (just processed) EFI_SIGNATURE_LIST object
|
|
|
e1263a |
in the signature database is "iter->offset".
|
|
|
e1263a |
|
|
|
e1263a |
- The size of the same is in "iter->esl->SignatureListSize".
|
|
|
e1263a |
|
|
|
e1263a |
- The size of the whole signature dabatase (containing the current
|
|
|
e1263a |
EFI_SIGNATURE_LIST) is in "iter->len".
|
|
|
e1263a |
|
|
|
e1263a |
Thus, we need to advance "iter->offset" by "iter->esl->SignatureListSize",
|
|
|
e1263a |
to reach the next EFI_SIGNATURE_LIST object.
|
|
|
e1263a |
|
|
|
e1263a |
While advancing, we must not exceed the whole signature database. In other
|
|
|
e1263a |
words, the (exclusive) end of the just processed EFI_SIGNATURE_LIST object
|
|
|
e1263a |
is required to precede, or equal, the (exclusive) end of the signature
|
|
|
e1263a |
database. Hence the "good" condition is:
|
|
|
e1263a |
|
|
|
e1263a |
iter->offset + iter->esl->SignatureListSize <= iter->len
|
|
|
e1263a |
|
|
|
e1263a |
The "bad" condition is the negation of the above:
|
|
|
e1263a |
|
|
|
e1263a |
iter->offset + iter->esl->SignatureListSize > iter->len
|
|
|
e1263a |
|
|
|
e1263a |
Because we don't trust "iter->esl->SignatureListSize" (since that was
|
|
|
e1263a |
simply read from the binary blob, not computed by ourselves), we don't
|
|
|
e1263a |
want to add to it or subtract from it (integer overflow!), we just want to
|
|
|
e1263a |
use it naked for comparison. So we subtract "iter->offset" from both
|
|
|
e1263a |
sides: "iter->offset" and "iter->len" are known-good because we've checked
|
|
|
e1263a |
and computed them all along, so we can perform integer operations on them.
|
|
|
e1263a |
After the subtraction, we have the following condition for *bad*:
|
|
|
e1263a |
|
|
|
e1263a |
iter->esl->SignatureListSize > iter->len - iter->offset
|
|
|
e1263a |
|
|
|
e1263a |
Another way to put the same condition, for *bad*, is to swing the sides
|
|
|
e1263a |
around the relop (giving a spin to the relop as well):
|
|
|
e1263a |
|
|
|
e1263a |
iter->len - iter->offset < iter->esl->SignatureListSize
|
|
|
e1263a |
|
|
|
e1263a |
The controlling expression in esl_iter_next() is just this, except for the
|
|
|
e1263a |
typo in the relational operator. Fix it.
|
|
|
e1263a |
|
|
|
e1263a |
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1508808
|
|
|
e1263a |
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
|
|
|
e1263a |
---
|
|
|
e1263a |
src/iter.c | 2 +-
|
|
|
e1263a |
1 file changed, 1 insertion(+), 1 deletion(-)
|
|
|
e1263a |
|
|
|
e1263a |
diff --git a/src/iter.c b/src/iter.c
|
|
|
e1263a |
index 45ee059e74c..f19166ab276 100644
|
|
|
e1263a |
--- a/src/iter.c
|
|
|
e1263a |
+++ b/src/iter.c
|
|
|
e1263a |
@@ -222,7 +222,7 @@ esl_iter_next(esl_iter *iter, efi_guid_t *type,
|
|
|
e1263a |
vprintf("Getting next EFI_SIGNATURE_LIST\n");
|
|
|
e1263a |
efi_guid_t type;
|
|
|
e1263a |
esl_get_type(iter, &type);
|
|
|
e1263a |
- if (iter->len - iter->offset > iter->esl->SignatureListSize) {
|
|
|
e1263a |
+ if (iter->len - iter->offset < iter->esl->SignatureListSize) {
|
|
|
e1263a |
warnx("EFI Signature List is malformed");
|
|
|
e1263a |
errx(1, "list has %zd bytes left, element is %"PRIu32" bytes",
|
|
|
e1263a |
iter->len - iter->offset,
|
|
|
e1263a |
--
|
|
|
e1263a |
2.29.2
|
|
|
e1263a |
|