From e2f757a5c78fce69faeeacb0850b886374c63b5f Mon Sep 17 00:00:00 2001 From: Paul Gardiner Date: Tue, 28 Aug 2018 14:11:49 +0100 Subject: 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. --- include/mupdf/pdf/document.h | 6 +++++- source/helpers/pkcs7/pkcs7-openssl.c | 30 ++++++++++++++++++++------- source/pdf/pdf-form.c | 39 ++++++++++++++++++++---------------- source/pdf/pdf-write.c | 30 ++++++++++++++++++++++----- 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); } } -- cgit v1.2.3