From 03b9f800b99b2f980e13fbc994d14bd8ec340c41 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Mon, 17 Oct 2016 16:16:17 -0400 Subject: [PATCH] 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 --- shim.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/shim.c b/shim.c index c69961b..cc3654e 100644 --- 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)) ||