generate_hash(): make check_size() set an error, and verify SecDir size.

Currently generate_hash() attempts to include any trailing data at the
end of the binary in the resulting digest, but it won't include such
data if the size computed is wrong because context->SecDir->Size is
invalid.  In this case the return code is EFI_SUCCESS, and the hash will
match any a binary as if the Attribute Certificate Table and anything
after it are missing.  This is wrong.

Signed-off-by: Peter Jones <pjones@redhat.com>

diff --git a/shim.c b/shim.c
index c69961b99c4b141ab5aa5b1ce402e0bbf8119236..cc3654e7b4f0014a2f1737d6b7150c17f3600545 100644 (file)
--- a/shim.c
+++ b/shim.c
@@ -686,12 +686,14 @@ static BOOLEAN secure_mode (void)
 #define check_size_line(data, datasize_in, hashbase, hashsize, l) ({   \
        if ((unsigned long)hashbase >                                   \
                        (unsigned long)data + datasize_in) {            \
+               status = EFI_INVALID_PARAMETER;                         \
                perror(L"shim.c:%d Invalid hash base 0x%016x\n", l,     \
                        hashbase);                                      \
                goto done;                                              \
        }                                                               \
        if ((unsigned long)hashbase + hashsize >                        \
                        (unsigned long)data + datasize_in) {            \
+               status = EFI_INVALID_PARAMETER;                         \
                perror(L"shim.c:%d Invalid hash size 0x%016x\n", l,     \
                        hashsize);                                      \
                goto done;                                              \
@@ -887,6 +889,13 @@ static EFI_STATUS generate_hash (char *data, unsigned int datasize_in,
        if (datasize > SumOfBytesHashed) {
                hashbase = data + SumOfBytesHashed;
                hashsize = datasize - context->SecDir->Size - SumOfBytesHashed;
+
+               if ((datasize - SumOfBytesHashed < context->SecDir->Size) ||
+                   (SumOfBytesHashed - hashsize != context->SecDir->VirtualAddress)) {
+                       perror(L"Malformed binary after Attribute Certificate Table\n");
+                       status = EFI_INVALID_PARAMETER;
+                       goto done;
+               }
                check_size(data, datasize_in, hashbase, hashsize);
 
                if (!(Sha256Update(sha256ctx, hashbase, hashsize)) ||