Bug 703100: Avoid encrypting (and extending) signature digests

When creating a new digital signature in a PDF MuPDF asks the
PKCS7 signer for the maximum digest size and adds a zeroed out
dummy signature as a /Contents string to the signature
dictionary. When MUPDF saves the document to a PDF file using
encryption it used to also encrypt the /Contents entry. For
AESv2/v3 encryption schemes strings will be extended by 16-31
bytes owing to an preceding iv string and trailing padding. Next
MuPDF completes unsaved signatures by locating their signature
dictionary objects in the PDF file, finding the /Contents entries
and overwriting their values with the an unencrypted signature
digests calculated by the PKCS7 signer according to the ranges
specified by each signatures /ByteRange entry.

For the PDF from the bug the initial zeroed out signature digest
was extended by 17 bytes when encrypted, thus the reserved size
for the /Contents entry was 17 bytes larger. When completing the
unsaved signatures MuPDF would previously, in pdf_write_digest(),
compare this reserved size to that of the unencrypted zeroed out
dummy signature. These differed by 17 bytes and
pdf_write_digest() errored out.

The check revealed that the zeroed out dummy signature digests
were written using encrypting and then patched without
encryption. Despite this the check is misguided because it
compares the encrypted and unencrypted lengths of the zeroed out
dummy signature digest. The check was meant to compare the
reserved space for the signature digest to the size of the digest
calculated by the PKCS7 signer.

This commit implements the correct approach which is to not
encrypt the initial dummy signature digest /Contents entry while
writing objects to the PDF file, and compare the size of the
digest returned by the PKCS7 signer to the reserved space in the
PDF file. The check has been modified to throw an exception if no
digest is received or if the digest is larger than the reserved
space. Should the digest be shorter than the reserved space,
it will be zeropadded at the end until it cover the entire space.

diff --git a/source/pdf/pdf-crypt.c b/source/pdf/pdf-crypt.c
index 6ccfe9e45b29a9fda72cd7baf697442a83e11ab3..de085f58bafd1c4a494e71f8e01a55a77630d311 100644 (file)
--- a/source/pdf/pdf-crypt.c
+++ b/source/pdf/pdf-crypt.c
@@ -1050,6 +1050,14 @@ pdf_compute_object_key(pdf_crypt *crypt, pdf_crypt_filter *cf, int num, int gen,
  * indirect references.
  */
 
+static int is_signature(fz_context *ctx, pdf_obj *obj)
+{
+       if (pdf_dict_get(ctx, obj, PDF_NAME(Type)) == PDF_NAME(Sig))
+               if (pdf_dict_get(ctx, obj, PDF_NAME(Contents)) && pdf_dict_get(ctx, obj, PDF_NAME(ByteRange)) && pdf_dict_get(ctx, obj, PDF_NAME(Filter)))
+                       return 1;
+       return 0;
+}
+
 static void
 pdf_crypt_obj_imp(fz_context *ctx, pdf_crypt *crypt, pdf_obj *obj, unsigned char *key, int keylen)
 {
@@ -1110,6 +1118,9 @@ pdf_crypt_obj_imp(fz_context *ctx, pdf_crypt *crypt, pdf_obj *obj, unsigned char
                int n = pdf_dict_len(ctx, obj);
                for (i = 0; i < n; i++)
                {
+                       if (pdf_dict_get_key(ctx, obj, i) == PDF_NAME(Contents) && is_signature(ctx, obj))
+                               continue;
+
                        pdf_crypt_obj_imp(ctx, crypt, pdf_dict_get_val(ctx, obj, i), key, keylen);
                }
        }

diff --git a/source/pdf/pdf-object.c b/source/pdf/pdf-object.c
index f5de80cb0ca36159cd99ab481c1e349fbb6eb5b0..c684fec44e1f15a93674f3e922a06dab6262874f 100644 (file)
--- a/source/pdf/pdf-object.c
+++ b/source/pdf/pdf-object.c
@@ -2043,6 +2043,14 @@ static void fmt_array(fz_context *ctx, struct fmt *fmt, pdf_obj *obj)
        }
 }
 
+static int is_signature(fz_context *ctx, pdf_obj *obj)
+{
+       if (pdf_dict_get(ctx, obj, PDF_NAME(Type)) ==  PDF_NAME(Sig))
+               if (pdf_dict_get(ctx, obj, PDF_NAME(Contents)) && pdf_dict_get(ctx, obj, PDF_NAME(ByteRange)) && pdf_dict_get(ctx, obj, PDF_NAME(Filter)))
+                       return 1;
+       return 0;
+}
+
 static void fmt_dict(fz_context *ctx, struct fmt *fmt, pdf_obj *obj)
 {
        int i, n;
@@ -2052,9 +2060,25 @@ static void fmt_dict(fz_context *ctx, struct fmt *fmt, pdf_obj *obj)
        if (fmt->tight) {
                fmt_puts(ctx, fmt, "<<");
                for (i = 0; i < n; i++) {
-                       fmt_obj(ctx, fmt, pdf_dict_get_key(ctx, obj, i));
+                       key = pdf_dict_get_key(ctx, obj, i);
+                       val = pdf_dict_get_val(ctx, obj, i);
+                       fmt_obj(ctx, fmt, key);
                        fmt_sep(ctx, fmt);
-                       fmt_obj(ctx, fmt, pdf_dict_get_val(ctx, obj, i));
+                       if (key == PDF_NAME(Contents) && is_signature(ctx, obj))
+                       {
+                               pdf_crypt *crypt = fmt->crypt;
+                               fz_try(ctx)
+                               {
+                                       fmt->crypt = NULL;
+                                       fmt_obj(ctx, fmt, val);
+                               }
+                               fz_always(ctx)
+                                       fmt->crypt = crypt;
+                               fz_catch(ctx)
+                                       fz_rethrow(ctx);
+                       }
+                       else
+                               fmt_obj(ctx, fmt, val);
                        fmt_sep(ctx, fmt);
                }
                fmt_puts(ctx, fmt, ">>");
@@ -2070,7 +2094,21 @@ static void fmt_dict(fz_context *ctx, struct fmt *fmt, pdf_obj *obj)
                        fmt_putc(ctx, fmt, ' ');
                        if (!pdf_is_indirect(ctx, val) && pdf_is_array(ctx, val))
                                fmt->indent ++;
-                       fmt_obj(ctx, fmt, val);
+                       if (key == PDF_NAME(Contents) && is_signature(ctx, obj))
+                       {
+                               pdf_crypt *crypt = fmt->crypt;
+                               fz_try(ctx)
+                               {
+                                       fmt->crypt = NULL;
+                                       fmt_obj(ctx, fmt, val);
+                               }
+                               fz_always(ctx)
+                                       fmt->crypt = crypt;
+                               fz_catch(ctx)
+                                       fz_rethrow(ctx);
+                       }
+                       else
+                               fmt_obj(ctx, fmt, val);
                        fmt_putc(ctx, fmt, '\n');
                        if (!pdf_is_indirect(ctx, val) && pdf_is_array(ctx, val))
                                fmt->indent --;

diff --git a/source/pdf/pdf-signature.c b/source/pdf/pdf-signature.c
index 5e174d66ed59301c8ad7b91ffdb09ae200c24848..5839f7573fc3c6cce0e9f30a74730a11ed9e7187 100644 (file)
--- a/source/pdf/pdf-signature.c
+++ b/source/pdf/pdf-signature.c
@@ -19,9 +19,7 @@ void pdf_write_digest(fz_context *ctx, fz_output *out, pdf_obj *byte_range, pdf_
        unsigned char *digest = NULL;
        size_t digest_len;
        pdf_obj *v = pdf_dict_get(ctx, field, PDF_NAME(V));
-       pdf_obj *c = pdf_dict_get(ctx, v, PDF_NAME(Contents));
        size_t len;
-       const char *s = pdf_to_string(ctx, c, &len);
        char *cstr = NULL;
 
        fz_var(stm);
@@ -33,9 +31,7 @@ void pdf_write_digest(fz_context *ctx, fz_output *out, pdf_obj *byte_range, pdf_
        if (hexdigest_length < 4)
                fz_throw(ctx, FZ_ERROR_GENERIC, "Bad parameters to pdf_write_digest");
 
-       digest_len = (hexdigest_length - 2) / 2;
-       if (len < digest_len)
-               fz_throw(ctx, FZ_ERROR_GENERIC, "Signature contents array smaller than digest");
+       len = (hexdigest_length - 2) / 2;
 
        fz_try(ctx)
        {
@@ -52,8 +48,12 @@ void pdf_write_digest(fz_context *ctx, fz_output *out, pdf_obj *byte_range, pdf_
                stm = fz_stream_from_output(ctx, out);
                in = fz_open_range_filter(ctx, stm, brange, brange_len);
 
-               digest = fz_malloc(ctx, digest_len);
-               digest_len = signer->create_digest(ctx, signer, in, digest, digest_len);
+               digest = fz_malloc(ctx, len);
+               digest_len = signer->create_digest(ctx, signer, in, digest, len);
+               if (digest_len == 0)
+                       fz_throw(ctx, FZ_ERROR_GENERIC, "signer provided no signature digest");
+               if (digest_len > len)
+                       fz_throw(ctx, FZ_ERROR_GENERIC, "signature digest larger than space for digest");
 
                fz_drop_stream(ctx, in);
                in = NULL;
@@ -61,13 +61,14 @@ void pdf_write_digest(fz_context *ctx, fz_output *out, pdf_obj *byte_range, pdf_
                stm = NULL;
 
                fz_seek_output(ctx, out, (int64_t)hexdigest_offset+1, SEEK_SET);
-
                cstr = fz_malloc(ctx, len);
 
-               for (z = 0; z < digest_len; z++)
-                       fz_write_printf(ctx, out, "%02x", digest[z]);
-               memcpy(cstr, digest, digest_len);
-               memcpy(cstr + digest_len, s + digest_len, len - digest_len);
+               for (z = 0; z < len; z++)
+               {
+                       int val = z < digest_len ? digest[z] : 0;
+                       fz_write_printf(ctx, out, "%02x", val);
+                       cstr[z] = val;
+               }
 
                pdf_dict_put_string(ctx, v, PDF_NAME(Contents), cstr, len);
        }