From 3998bcabb2a4abb665f90cf50cc2a6efa85bbc8e Mon Sep 17 00:00:00 2001 From: olszomal Date: Wed, 28 Feb 2024 15:12:20 +0100 Subject: [PATCH] Simplify BIO chain free up and FILE_FORMAT_CTX cleanup --- appx.c | 13 ++++--------- cab.c | 13 ++++--------- cat.c | 13 ++++--------- msi.c | 13 ++++--------- osslsigncode.c | 23 +++++++++++++++-------- osslsigncode.h | 4 ++-- pe.c | 13 ++++--------- script.c | 14 ++++---------- 8 files changed, 41 insertions(+), 65 deletions(-) diff --git a/appx.c b/appx.c index a98cdaa..00265f3 100644 --- a/appx.c +++ b/appx.c @@ -256,8 +256,8 @@ static int appx_remove_pkcs7(FILE_FORMAT_CTX *ctx, BIO *hash, BIO *outdata); static int appx_process_data(FILE_FORMAT_CTX *ctx, BIO *hash, BIO *outdata); static PKCS7 *appx_pkcs7_signature_new(FILE_FORMAT_CTX *ctx, BIO *hash); static int appx_append_pkcs7(FILE_FORMAT_CTX *ctx, BIO *outdata, PKCS7 *p7); -static BIO *appx_bio_free(BIO *hash, BIO *outdata); -static void appx_ctx_cleanup(FILE_FORMAT_CTX *ctx, BIO *hash, BIO *outdata); +static void appx_bio_free(BIO *hash, BIO *outdata); +static void appx_ctx_cleanup(FILE_FORMAT_CTX *ctx); FILE_FORMAT file_format_appx = { .ctx_new = appx_ctx_new, @@ -757,11 +757,10 @@ static int appx_append_pkcs7(FILE_FORMAT_CTX *ctx, BIO *outdata, PKCS7 *p7) * [out] outdata: outdata file BIO * [returns] none */ -static BIO *appx_bio_free(BIO *hash, BIO *outdata) +static void appx_bio_free(BIO *hash, BIO *outdata) { BIO_free_all(outdata); BIO_free_all(hash); - return NULL; /* OK */ } /* @@ -771,12 +770,8 @@ static BIO *appx_bio_free(BIO *hash, BIO *outdata) * [in] outdata: outdata file BIO * [returns] none */ -static void appx_ctx_cleanup(FILE_FORMAT_CTX *ctx, BIO *hash, BIO *outdata) +static void appx_ctx_cleanup(FILE_FORMAT_CTX *ctx) { - if (outdata) { - BIO_free_all(hash); - BIO_free_all(outdata); - } freeZip(ctx->appx_ctx->zip); OPENSSL_free(ctx->appx_ctx->calculatedBMHash); OPENSSL_free(ctx->appx_ctx->calculatedCTHash); diff --git a/cab.c b/cab.c index 7388ef9..684469a 100644 --- a/cab.c +++ b/cab.c @@ -54,8 +54,8 @@ static int cab_process_data(FILE_FORMAT_CTX *ctx, BIO *hash, BIO *outdata); static PKCS7 *cab_pkcs7_signature_new(FILE_FORMAT_CTX *ctx, BIO *hash); static int cab_append_pkcs7(FILE_FORMAT_CTX *ctx, BIO *outdata, PKCS7 *p7); static void cab_update_data_size(FILE_FORMAT_CTX *ctx, BIO *outdata, PKCS7 *p7); -static BIO *cab_bio_free(BIO *hash, BIO *outdata); -static void cab_ctx_cleanup(FILE_FORMAT_CTX *ctx, BIO *hash, BIO *outdata); +static void cab_bio_free(BIO *hash, BIO *outdata); +static void cab_ctx_cleanup(FILE_FORMAT_CTX *ctx); static int cab_is_detaching_supported(void); FILE_FORMAT file_format_cab = { @@ -599,13 +599,11 @@ static void cab_update_data_size(FILE_FORMAT_CTX *ctx, BIO *outdata, PKCS7 *p7) * [out] outdata: outdata file BIO (unused) * [returns] none */ -static BIO *cab_bio_free(BIO *hash, BIO *outdata) +static void cab_bio_free(BIO *hash, BIO *outdata) { /* squash the unused parameter warning */ (void)outdata; - BIO_free_all(hash); - return NULL; } /* @@ -616,11 +614,8 @@ static BIO *cab_bio_free(BIO *hash, BIO *outdata) * [in] outdata: outdata file BIO * [returns] none */ -static void cab_ctx_cleanup(FILE_FORMAT_CTX *ctx, BIO *hash, BIO *outdata) +static void cab_ctx_cleanup(FILE_FORMAT_CTX *ctx) { - if (outdata) { - BIO_free_all(hash); - } unmap_file(ctx->options->indata, ctx->cab_ctx->fileend); OPENSSL_free(ctx->cab_ctx); OPENSSL_free(ctx); diff --git a/cat.c b/cat.c index 45de700..a8eecb4 100644 --- a/cat.c +++ b/cat.c @@ -40,8 +40,8 @@ static int cat_verify_digests(FILE_FORMAT_CTX *ctx, PKCS7 *p7); static PKCS7 *cat_pkcs7_extract(FILE_FORMAT_CTX *ctx); static PKCS7 *cat_pkcs7_signature_new(FILE_FORMAT_CTX *ctx, BIO *hash); static int cat_append_pkcs7(FILE_FORMAT_CTX *ctx, BIO *outdata, PKCS7 *p7); -static BIO *cat_bio_free(BIO *hash, BIO *outdata); -static void cat_ctx_cleanup(FILE_FORMAT_CTX *ctx, BIO *hash, BIO *outdata); +static void cat_bio_free(BIO *hash, BIO *outdata); +static void cat_ctx_cleanup(FILE_FORMAT_CTX *ctx); FILE_FORMAT file_format_cat = { .ctx_new = cat_ctx_new, @@ -192,13 +192,11 @@ static int cat_append_pkcs7(FILE_FORMAT_CTX *ctx, BIO *outdata, PKCS7 *p7) * [out] outdata: outdata file BIO (unused) * [returns] none */ -static BIO *cat_bio_free(BIO *hash, BIO *outdata) +static void cat_bio_free(BIO *hash, BIO *outdata) { /* squash the unused parameter warning */ (void)outdata; - BIO_free_all(hash); - return NULL; } /* @@ -209,11 +207,8 @@ static BIO *cat_bio_free(BIO *hash, BIO *outdata) * [in] outdata: outdata file BIO * [returns] none */ -static void cat_ctx_cleanup(FILE_FORMAT_CTX *ctx, BIO *hash, BIO *outdata) +static void cat_ctx_cleanup(FILE_FORMAT_CTX *ctx) { - if (outdata) { - BIO_free_all(hash); - } unmap_file(ctx->options->indata, ctx->cat_ctx->fileend); PKCS7_free(ctx->cat_ctx->p7); OPENSSL_free(ctx->cat_ctx); diff --git a/msi.c b/msi.c index 2b43f2f..eb55af4 100644 --- a/msi.c +++ b/msi.c @@ -201,8 +201,8 @@ static int msi_remove_pkcs7(FILE_FORMAT_CTX *ctx, BIO *hash, BIO *outdata); static int msi_process_data(FILE_FORMAT_CTX *ctx, BIO *hash, BIO *outdata); static PKCS7 *msi_pkcs7_signature_new(FILE_FORMAT_CTX *ctx, BIO *hash); static int msi_append_pkcs7(FILE_FORMAT_CTX *ctx, BIO *outdata, PKCS7 *p7); -static BIO *msi_bio_free(BIO *hash, BIO *outdata); -static void msi_ctx_cleanup(FILE_FORMAT_CTX *ctx, BIO *hash, BIO *outdata); +static void msi_bio_free(BIO *hash, BIO *outdata); +static void msi_ctx_cleanup(FILE_FORMAT_CTX *ctx); static int msi_is_detaching_supported(void); FILE_FORMAT file_format_msi = { @@ -671,11 +671,10 @@ static int msi_append_pkcs7(FILE_FORMAT_CTX *ctx, BIO *outdata, PKCS7 *p7) * [out] outdata: outdata file BIO * [returns] none */ -static BIO *msi_bio_free(BIO *hash, BIO *outdata) +static void msi_bio_free(BIO *hash, BIO *outdata) { BIO_free_all(hash); BIO_free_all(outdata); - return NULL; } /* @@ -686,12 +685,8 @@ static BIO *msi_bio_free(BIO *hash, BIO *outdata) * [out] outdata: outdata file BIO * [returns] none */ -static void msi_ctx_cleanup(FILE_FORMAT_CTX *ctx, BIO *hash, BIO *outdata) +static void msi_ctx_cleanup(FILE_FORMAT_CTX *ctx) { - if (outdata) { - BIO_free_all(hash); - BIO_free_all(outdata); - } unmap_file(ctx->options->indata, ctx->msi_ctx->fileend); msi_file_free(ctx->msi_ctx->msi); msi_dirent_free(ctx->msi_ctx->dirent); diff --git a/osslsigncode.c b/osslsigncode.c index 13f09e6..6fd7b03 100644 --- a/osslsigncode.c +++ b/osslsigncode.c @@ -2743,7 +2743,7 @@ static int verify_signed_file(FILE_FORMAT_CTX *ctx, GLOBAL_OPTIONS *options) return 1; /* FAILED */ } p7 = cat_ctx->format->pkcs7_extract(cat_ctx); - cat_ctx->format->ctx_cleanup(cat_ctx, NULL, NULL); + cat_ctx->format->ctx_cleanup(cat_ctx); OPENSSL_free(cat_options); } else { if (!ctx->format->pkcs7_extract) { @@ -2970,11 +2970,11 @@ static int check_attached_data(GLOBAL_OPTIONS *options) } if (verify_signed_file(ctx, tmp_options)) { printf("Signature mismatch\n"); - ctx->format->ctx_cleanup(ctx, NULL, NULL); + ctx->format->ctx_cleanup(ctx); OPENSSL_free(tmp_options); return 1; /* Failed */ } - ctx->format->ctx_cleanup(ctx, NULL, NULL); + ctx->format->ctx_cleanup(ctx); OPENSSL_free(tmp_options); return 0; /* OK */ } @@ -4440,6 +4440,7 @@ int main(int argc, char **argv) } BIO_free_all(hash); BIO_free_all(outdata); + outdata = NULL; ret = 1; /* FAILED */ DO_EXIT_0("Initialization error or unsupported input file type.\n"); } @@ -4580,7 +4581,8 @@ int main(int argc, char **argv) skip_signing: if (ctx->format->bio_free) { - outdata = ctx->format->bio_free(hash, outdata); + ctx->format->bio_free(hash, outdata); + outdata = NULL; } if (!ret && options.cmd == CMD_ATTACH) { ret = check_attached_data(&options); @@ -4594,12 +4596,17 @@ skip_signing: } err_cleanup: - if (outdata && options.outfile) { - /* unlink outfile */ - remove_file(options.outfile); + if (outdata) { + if (options.outfile) { + /* unlink outfile */ + remove_file(options.outfile); + } + if (hash) + BIO_free(hash); + BIO_free(outdata); } if (ctx && ctx->format->ctx_cleanup) { - ctx->format->ctx_cleanup(ctx, hash, outdata); + ctx->format->ctx_cleanup(ctx); } #if OPENSSL_VERSION_NUMBER>=0x30000000L providers_cleanup(); diff --git a/osslsigncode.h b/osslsigncode.h index 6bcf436..4fad3e5 100644 --- a/osslsigncode.h +++ b/osslsigncode.h @@ -523,8 +523,8 @@ struct file_format_st { PKCS7 *(*pkcs7_signature_new) (FILE_FORMAT_CTX *ctx, BIO *hash); int (*append_pkcs7) (FILE_FORMAT_CTX *ctx, BIO *outdata, PKCS7 *p7); void (*update_data_size) (FILE_FORMAT_CTX *data, BIO *outdata, PKCS7 *p7); - BIO *(*bio_free) (BIO *hash, BIO *outdata); - void (*ctx_cleanup) (FILE_FORMAT_CTX *ctx, BIO *hash, BIO *outdata); + void (*bio_free) (BIO *hash, BIO *outdata); + void (*ctx_cleanup) (FILE_FORMAT_CTX *ctx); int (*is_detaching_supported) (void); }; diff --git a/pe.c b/pe.c index fd4261b..613d0f4 100644 --- a/pe.c +++ b/pe.c @@ -56,8 +56,8 @@ static int pe_process_data(FILE_FORMAT_CTX *ctx, BIO *hash, BIO *outdata); static PKCS7 *pe_pkcs7_signature_new(FILE_FORMAT_CTX *ctx, BIO *hash); static int pe_append_pkcs7(FILE_FORMAT_CTX *ctx, BIO *outdata, PKCS7 *p7); static void pe_update_data_size(FILE_FORMAT_CTX *ctx, BIO *outdata, PKCS7 *p7); -static BIO *pe_bio_free(BIO *hash, BIO *outdata); -static void pe_ctx_cleanup(FILE_FORMAT_CTX *ctx, BIO *hash, BIO *outdata); +static void pe_bio_free(BIO *hash, BIO *outdata); +static void pe_ctx_cleanup(FILE_FORMAT_CTX *ctx); static int pe_is_detaching_supported(void); FILE_FORMAT file_format_pe = { @@ -496,13 +496,11 @@ static void pe_update_data_size(FILE_FORMAT_CTX *ctx, BIO *outdata, PKCS7 *p7) * [out] outdata: outdata file BIO (unused) * [returns] none */ -static BIO *pe_bio_free(BIO *hash, BIO *outdata) +static void pe_bio_free(BIO *hash, BIO *outdata) { /* squash the unused parameter warning */ (void)outdata; - BIO_free_all(hash); - return NULL; } /* @@ -513,11 +511,8 @@ static BIO *pe_bio_free(BIO *hash, BIO *outdata) * [in] outdata: outdata file BIO * [returns] none */ -static void pe_ctx_cleanup(FILE_FORMAT_CTX *ctx, BIO *hash, BIO *outdata) +static void pe_ctx_cleanup(FILE_FORMAT_CTX *ctx) { - if (outdata) { - BIO_free_all(hash); - } unmap_file(ctx->options->indata, ctx->pe_ctx->fileend); OPENSSL_free(ctx->pe_ctx); OPENSSL_free(ctx); diff --git a/script.c b/script.c index 5706b65..46fb1b9 100644 --- a/script.c +++ b/script.c @@ -62,8 +62,8 @@ static int script_remove_pkcs7(FILE_FORMAT_CTX *ctx, BIO *hash, BIO *outdata); static int script_process_data(FILE_FORMAT_CTX *ctx, BIO *hash, BIO *outdata); static PKCS7 *script_pkcs7_signature_new(FILE_FORMAT_CTX *ctx, BIO *hash); static int script_append_pkcs7(FILE_FORMAT_CTX *ctx, BIO *outdata, PKCS7 *p7); -static BIO *script_bio_free(BIO *hash, BIO *outdata); -static void script_ctx_cleanup(FILE_FORMAT_CTX *ctx, BIO *hash, BIO *outdata); +static void script_bio_free(BIO *hash, BIO *outdata); +static void script_ctx_cleanup(FILE_FORMAT_CTX *ctx); static int script_is_detaching_supported(void); FILE_FORMAT file_format_script = { @@ -575,12 +575,10 @@ cleanup: * [out] outdata: outdata file BIO * [returns] none */ -static BIO *script_bio_free(BIO *hash, BIO *outdata) +static void script_bio_free(BIO *hash, BIO *outdata) { BIO_free_all(hash); BIO_free_all(outdata); - /* FIXME: why doesn't the function return void instead of BIO *? */ - return NULL; } /* @@ -590,12 +588,8 @@ static BIO *script_bio_free(BIO *hash, BIO *outdata) * [out] outdata: outdata file BIO * [returns] none */ -static void script_ctx_cleanup(FILE_FORMAT_CTX *ctx, BIO *hash, BIO *outdata) +static void script_ctx_cleanup(FILE_FORMAT_CTX *ctx) { - if (outdata) { - BIO_free_all(hash); - BIO_free_all(outdata); - } unmap_file(ctx->options->indata, ctx->script_ctx->fileend); OPENSSL_free(ctx->script_ctx); OPENSSL_free(ctx);