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