Avoid use of fixed size buffers for the digest when document signing

The signer object now has an extra method that informs the caller of
the maximum size the digest might be. This is used to allocate space for
the digest within the file and to size some of the buffers used in the
code.

The openssl-based inplementation of the signer object has been updated to
perform a test digest generation so as to find the size needed. We
believe that the digest size is indendent of the hashed data.

4 files changed, 75 insertions, 30 deletions

diff --git a/include/mupdf/pdf/document.h b/include/mupdf/pdf/document.h
index 9e9121df..7d2110b7 100644
--- a/include/mupdf/pdf/document.h
+++ b/include/mupdf/pdf/document.h
@@ -556,7 +556,10 @@ typedef pdf_pkcs7_designated_name *(pdf_pkcs7_designated_name_fn)(pdf_pkcs7_sign
 /* Free the resources associated with previously obtained designated name information */
 typedef void (pdf_pkcs7_drop_designated_name_fn)(pdf_pkcs7_signer *signer, pdf_pkcs7_designated_name *name);
 
-/* Create a signature based on ranges of bytes drawn from a steam */
+/* Predict the size of the digest. The actual digest returned by create_digest will be no greater in size */
+typedef int (pdf_pkcs7_max_digest_size_fn)(pdf_pkcs7_signer *signer);
+
+/* Create a signature based on ranges of bytes drawn from a stream */
 typedef int (pdf_pkcs7_create_digest_fn)(pdf_pkcs7_signer *signer, fz_stream *in, unsigned char *digest, int *digest_len);
 
 struct pdf_pkcs7_signer_s
@@ -565,6 +568,7 @@ struct pdf_pkcs7_signer_s
 	pdf_pkcs7_drop_fn *drop;
 	pdf_pkcs7_designated_name_fn *designated_name;
 	pdf_pkcs7_drop_designated_name_fn *drop_designated_name;
+	pdf_pkcs7_max_digest_size_fn *max_digest_size;
 	pdf_pkcs7_create_digest_fn *create_digest;
 };
 
diff --git a/source/helpers/pkcs7/pkcs7-openssl.c b/source/helpers/pkcs7/pkcs7-openssl.c
index 1468b7e1..58dbf65f 100644
--- a/source/helpers/pkcs7/pkcs7-openssl.c
+++ b/source/helpers/pkcs7/pkcs7-openssl.c
@@ -633,10 +633,12 @@ static int signer_create_digest(pdf_pkcs7_signer *signer, fz_stream *in, unsigne
 	unsigned char *p7_ptr;
 	int p7_len;
 
-	bdata = BIO_new_stream(ctx, in);
-	if (bdata == NULL)
-		goto exit;
-
+	if (in != NULL)
+	{
+		bdata = BIO_new_stream(ctx, in);
+		if (bdata == NULL)
+			goto exit;
+	}
 
 	p7 = PKCS7_new();
 	if (p7 == NULL)
@@ -657,7 +659,7 @@ static int signer_create_digest(pdf_pkcs7_signer *signer, fz_stream *in, unsigne
 	if (bp7in == NULL)
 		goto exit;
 
-	while(1)
+	while(bdata) /* bdata knowingly not changed in the loop */
 	{
 		char buf[4096];
 		int n = BIO_read(bdata, buf, sizeof(buf));
@@ -677,10 +679,12 @@ static int signer_create_digest(pdf_pkcs7_signer *signer, fz_stream *in, unsigne
 		goto exit;
 
 	p7_len = BIO_get_mem_data(bp7, &p7_ptr);
-	if (p7_len > *digest_len)
+	if (digest && p7_len > *digest_len)
 		goto exit;
 
-	memcpy(digest, p7_ptr, p7_len);
+	if (digest)
+		memcpy(digest, p7_ptr, p7_len);
+
 	*digest_len = p7_len;
 	res = 1;
 
@@ -692,6 +696,17 @@ exit:
 	return res;
 }
 
+static int max_digest_size(pdf_pkcs7_signer *signer)
+{
+	/* Perform a test digest generation to find the required size. Size
+	 * is assumed independent of data being hashed */
+	int digest_len = 0;
+
+	signer_create_digest(signer, NULL, NULL, &digest_len);
+
+	return digest_len;
+}
+
 pdf_pkcs7_signer *pkcs7_openssl_read_pfx(fz_context *ctx, const char *pfile, const char *pw)
 {
 	BIO *pfxbio = NULL;
@@ -710,6 +725,7 @@ pdf_pkcs7_signer *pkcs7_openssl_read_pfx(fz_context *ctx, const char *pfile, con
 		signer->base.drop = drop_signer;
 		signer->base.designated_name = signer_designated_name;
 		signer->base.drop_designated_name = signer_drop_designated_name;
+		signer->base.max_digest_size = max_digest_size;
 		signer->base.create_digest = signer_create_digest;
 		signer->ctx = ctx;
 		signer->refs = 1;
diff --git a/source/pdf/pdf-form.c b/source/pdf/pdf-form.c
index af0f572d..d5dad22b 100644
--- a/source/pdf/pdf-form.c
+++ b/source/pdf/pdf-form.c
@@ -1289,40 +1289,45 @@ void pdf_signature_set_value(fz_context *ctx, pdf_document *doc, pdf_obj *field,
 	int vnum;
 	pdf_obj *byte_range;
 	pdf_obj *contents;
-	char buf[2048];
-
-	memset(buf, 0, sizeof(buf));
+	int max_digest_size;
+	char *buf = NULL;
 
 	vnum = pdf_create_object(ctx, doc);
 	indv = pdf_new_indirect(ctx, doc, vnum, 0);
 	pdf_dict_put_drop(ctx, field, PDF_NAME(V), indv);
 
+	max_digest_size = signer->max_digest_size(signer);
+
 	fz_var(v);
+	fz_var(buf);
 	fz_try(ctx)
 	{
 		v = pdf_new_dict(ctx, doc, 4);
 		pdf_update_object(ctx, doc, vnum, v);
+
+		buf = fz_calloc(ctx, max_digest_size, 1);
+
+		byte_range = pdf_new_array(ctx, doc, 4);
+		pdf_dict_put_drop(ctx, v, PDF_NAME(ByteRange), byte_range);
+
+		contents = pdf_new_string(ctx, buf, max_digest_size);
+		pdf_dict_put_drop(ctx, v, PDF_NAME(Contents), contents);
+
+		pdf_dict_put(ctx, v, PDF_NAME(Filter), PDF_NAME(Adobe_PPKLite));
+		pdf_dict_put(ctx, v, PDF_NAME(SubFilter), PDF_NAME(adbe_pkcs7_detached));
+
+		/* Record details within the document structure so that contents
+		* and byte_range can be updated with their correct values at
+		* saving time */
+		pdf_xref_store_unsaved_signature(ctx, doc, field, signer);
 	}
 	fz_always(ctx)
 	{
 		pdf_drop_obj(ctx, v);
+		fz_free(ctx, buf);
 	}
 	fz_catch(ctx)
 	{
 		fz_rethrow(ctx);
 	}
-
-	byte_range = pdf_new_array(ctx, doc, 4);
-	pdf_dict_put_drop(ctx, v, PDF_NAME(ByteRange), byte_range);
-
-	contents = pdf_new_string(ctx, buf, sizeof(buf));
-	pdf_dict_put_drop(ctx, v, PDF_NAME(Contents), contents);
-
-	pdf_dict_put(ctx, v, PDF_NAME(Filter), PDF_NAME(Adobe_PPKLite));
-	pdf_dict_put(ctx, v, PDF_NAME(SubFilter), PDF_NAME(adbe_pkcs7_detached));
-
-	/* Record details within the document structure so that contents
-	 * and byte_range can be updated with their correct values at
-	 * saving time */
-	pdf_xref_store_unsaved_signature(ctx, doc, field, signer);
 }
diff --git a/source/pdf/pdf-write.c b/source/pdf/pdf-write.c
index 26fcd9d1..227258d8 100644
--- a/source/pdf/pdf-write.c
+++ b/source/pdf/pdf-write.c
@@ -13,6 +13,8 @@
 /* #define DEBUG_HEAP_SORT */
 /* #define DEBUG_WRITING */
 
+#define SIG_EXTRAS_SIZE (1024)
+
 typedef struct pdf_write_state_s pdf_write_state;
 
 /*
@@ -2627,12 +2629,14 @@ static void presize_unsaved_signature_byteranges(fz_context *ctx, pdf_document *
 static void complete_signatures(fz_context *ctx, pdf_document *doc, pdf_write_state *opts)
 {
 	pdf_unsaved_sig *usig;
-	char buf[5120];
+	char *buf = NULL;
+	int buf_size;
 	int s;
 	int i;
 	int last_end;
 	fz_stream *stm = NULL;
 	fz_var(stm);
+	fz_var(buf);
 
 	fz_try(ctx)
 	{
@@ -2643,6 +2647,18 @@ static void complete_signatures(fz_context *ctx, pdf_document *doc, pdf_write_st
 			if (xref->unsaved_sigs)
 			{
 				pdf_obj *byte_range;
+				buf_size = 0;
+
+				for (usig = xref->unsaved_sigs; usig; usig = usig->next)
+				{
+					int size = usig->signer->max_digest_size(usig->signer);
+
+					buf_size = fz_maxi(buf_size, size);
+				}
+
+				buf_size = buf_size * 2 + SIG_EXTRAS_SIZE;
+
+				buf = fz_calloc(ctx, buf_size, 1);
 
 				stm = fz_stream_from_output(ctx, opts->out);
 				/* Locate the byte ranges and contents in the saved file */
@@ -2651,8 +2667,8 @@ static void complete_signatures(fz_context *ctx, pdf_document *doc, pdf_write_st
 					char *bstr, *cstr, *fstr;
 					int pnum = pdf_obj_parent_num(ctx, pdf_dict_getl(ctx, usig->field, PDF_NAME(V), PDF_NAME(ByteRange), NULL));
 					fz_seek(ctx, stm, opts->ofs_list[pnum], SEEK_SET);
-					(void)fz_read(ctx, stm, (unsigned char *)buf, sizeof(buf));
-					buf[sizeof(buf)-1] = 0;
+					(void)fz_read(ctx, stm, (unsigned char *)buf, buf_size);
+					buf[buf_size-1] = 0;
 
 					bstr = strstr(buf, "/ByteRange");
 					cstr = strstr(buf, "/Contents");
@@ -2690,8 +2706,8 @@ static void complete_signatures(fz_context *ctx, pdf_document *doc, pdf_write_st
 					pdf_dict_putl_drop(ctx, usig->field, pdf_copy_array(ctx, byte_range), PDF_NAME(V), PDF_NAME(ByteRange), NULL);
 
 				/* Write the byte range into buf, padding with spaces*/
-				i = pdf_sprint_obj(ctx, buf, sizeof(buf), byte_range, 1);
-				memset(buf+i, ' ', sizeof(buf)-i);
+				i = pdf_sprint_obj(ctx, buf, buf_size, byte_range, 1);
+				memset(buf+i, ' ', buf_size-i);
 
 				/* Write the byte range to the file */
 				for (usig = xref->unsaved_sigs; usig; usig = usig->next)
@@ -2714,12 +2730,16 @@ static void complete_signatures(fz_context *ctx, pdf_document *doc, pdf_write_st
 				}
 
 				xref->unsaved_sigs_end = NULL;
+
+				fz_free(ctx, buf);
+				buf = NULL;
 			}
 		}
 	}
 	fz_catch(ctx)
 	{
 		fz_drop_stream(ctx, stm);
+		fz_free(ctx, buf);
 		fz_rethrow(ctx);
 	}
 }