Package: pesign Version: 0.112-4 Severity: important Tags: upstream patch I'm wanting to use efisiglist to generate a dbx list in our shim package at build time. Unfortunately, I've just found (after a lot of debugging) that efisiglist gets things wrong and produces malformed output. I've fixed it - see attached patch.
About to push this upstream too, but adding a bug here to help track things. -- System Information: Debian Release: buster/sid APT prefers testing APT policy: (500, 'testing'), (500, 'stable') Architecture: amd64 (x86_64) Kernel: Linux 4.19.0-4-amd64 (SMP w/4 CPU cores) Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8), LANGUAGE=en_GB:en (charmap=UTF-8) Shell: /bin/sh linked to /usr/bin/dash Init: systemd (via /run/systemd/system) LSM: AppArmor: enabled Versions of packages pesign depends on: ii coolkey 1.1.0-13+b1 ii libc6 2.28-8 ii libefivar1 37-2 ii libnspr4 2:4.20-1 ii libnss3 2:3.42.1-1 ii libnss3-tools 2:3.42.1-1 ii libpopt0 1.16-12 ii libuuid1 2.33.1-0.1 ii opensc 0.19.0-1 pesign recommends no packages. pesign suggests no packages. -- no debconf information
Description: Fix bugs in efisiglist * Fix handling of structure sizes in a couple of places, so that memory copies etc. work properly. * In signature_list_realize(), don't simply copy the *sl struct over the *esl struct - they're *not* exactly the same! Copy fields by hand. Author: Steve McIntyre <93...@debian.org> --- --- pesign-0.112.orig/src/siglist.c +++ pesign-0.112/src/siglist.c @@ -77,12 +77,14 @@ static struct sig_type sig_types[] = { }; static int num_sig_types = sizeof (sig_types) / sizeof (struct sig_type); +/* How much space does a signature list entry take? Count the space + * for the GUID, then the signature/cert itself. */ static int32_t get_sig_type_size(const efi_guid_t *sig_type) { for (int i = 0; i < num_sig_types; i++) { if (!memcmp(sig_type, sig_types[i].type, sizeof (*sig_type))) - return sig_types[i].size; + return sig_types[i].size + sizeof(efi_guid_t); } return -1; } @@ -99,7 +101,7 @@ signature_list_new(const efi_guid_t *Sig return NULL; sl->SignatureType = SignatureType; - sl->SignatureSize = size + sizeof (efi_guid_t); + sl->SignatureSize = size; sl->SignatureListSize = sizeof (struct efi_signature_list); return sl; @@ -137,11 +139,21 @@ signature_list_add_sig(signature_list *s sl->realized = NULL; } + /* The sigsize passed in by the caller is just enough for + * their object (hash or cert). But sl->SignatureSize includes + * the size of the GUID before the hash/cert itself, so + * account for that too here. */ + sigsize += sizeof (efi_guid_t); + + /* If we're adding an x509 cert onto the end of a list of + * hashes, we will need to resize the list entries to cope */ if (!efi_guid_cmp(sl->SignatureType, &efi_guid_x509_cert)) { if (sigsize > sl->SignatureSize) - resize_entries(sl, sigsize + sizeof (efi_guid_t)); + resize_entries(sl, sigsize); } else if (sigsize != (unsigned long long)get_sig_type_size(sl->SignatureType)) { + + /* size mismatch - error out */ char *guidname = NULL; int rc = efi_guid_to_id_guid(sl->SignatureType, &guidname); if (rc < 0) { @@ -217,7 +229,10 @@ signature_list_realize(signature_list *s return -1; esl = ret; - memcpy(esl, sl, sizeof (*esl)); + memcpy(&esl->SignatureType, sl->SignatureType, sizeof *sl->SignatureType); + esl->SignatureListSize = sl->SignatureListSize; + esl->SignatureHeaderSize = 0; + esl->SignatureSize = sl->SignatureSize; uint8_t *pos = ret + sizeof (*esl); for (int i = 0; i < count; i++) {