Removing signature does not produce the original unsigned binary

[Blarse]

Hi,

I've found a bug in signature remove code: pesign would exit before removing signature if you don't specify signum explicitly with -u option.

The patch is trivial:

diff --git a/src/file_pe.c b/src/file_pe.c
index 805e614..407d27d 100644
--- a/src/file_pe.c
+++ b/src/file_pe.c
@@ -231,6 +231,8 @@ pe_handle_action(pesign_context *ctxp, int action, int padding)
                        open_input(ctxp);
                        open_output(ctxp);
                        close_input(ctxp);
+                       if (ctxp->signum < 0)
+                               ctxp->signum = 0;
                        if(ctxp->signum < 0 ||
                           ctxp->signum >= ctxp->cms_ctx->num_signatures) {
                                warnx("Invalid signature number %d.",

By a lucky coincidence original code did exactly what was expected. open_output function created a copy of the input file and then did pe_clearcert after which pesign exited with error.

But fixing this bug uncovered that continuing REMOVE_SIGNATURE action calls close_output->finalize_signatures->implant_cert_list->pe_alloccert which populates Security Data Directory with zero size data:

dd->certs.virtual_address = compute_file_addr(pe, addr);
dd->certs.size += size;

I would assume that somewhere in this call chain should be a check for size == 0 but not sure exactly where.

The other issue is that removing signature does not produce the original file because of the alignment:

@@ -57558,5 +57558,5 @@
 000e32a0  00 6c 6f 61 64 5f 6f 70  74 69 6f 6e 73 00 50 4b  |.load_options.PK|
 000e32b0  45 59 5f 55 53 41 47 45  5f 50 45 52 49 4f 44 5f  |EY_USAGE_PERIOD_|
 000e32c0  69 74 00 58 35 30 39 5f  4e 41 4d 45 5f 45 4e 54  |it.X509_NAME_ENT|
-000e32d0  52 59 5f 69 74 00                                 |RY_it.|
-000e32d6
+000e32d0  52 59 5f 69 74 00 00 00                           |RY_it...|
+000e32d8