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++) {

Reply via email to