mtrojnar / osslsigncode

OpenSSL based Authenticode signing for PE/MSI/Java CAB files
Other
731 stars 124 forks source link

0Kb file if input file is the same as output file #313

Closed Niaxor closed 7 months ago

Niaxor commented 8 months ago

It seems that osslsigncode doesn't support setting your input file as the same as the output file (to overwrite the input file).

However, this seems to be handled ungracefully, it results in the error:

Unrecognized file type - file is too short: <input file.exe> and leaves the input/output file as a 0kb file.

Support for overwriting the input file is a pretty handy feature since signing an executable is generally one step amongst many others as part of a build pipeline, but at the least if support is not intended I feel there should be a dedicated error message, as this may confuse some.

westyles commented 8 months ago

It would be convenient if it were possible to specify only -in without -out, so that the result would be in the same file. Or possibility to specify the same file in -in and -out.

mtrojnar commented 8 months ago

@olszomal Can you take a look? While refusing of in-place signing attempts is the intended behavior, removing the content of input file looks like a regression of a bug we fixed a long time ago.

olszomal commented 8 months ago
Usage: ./osslsigncode [ sign ] (...)
            [ -in ] <infile> [-out ] <outfile>

osslsigncode opens the input file for reading, and the output file for writing. It would overwrite its own input if the same file is was used for input and output.

The workaround is either to:

Niaxor commented 8 months ago

Correct me if I'm wrong but won't deleting the output file just result in a missing input file and still a super confused user?

As far as I'm concerned the issue here is two-fold:

  1. User isn't stopped or warned that they are doing something unsupported
  2. Input file is mangled

I think currently both of these issues will still be present.

A dedicated error message and then early exit of signing attempt would be much better imo

olszomal commented 8 months ago

Here is a dedicated error message:

./osslsigncode sign -in file.exe -out file.exe (...)
Failed to create file: Testing/files/file.exe
Niaxor commented 8 months ago

My bad! I may have taken the pr name at face value.

I'll test the PR tomorrow - thanks for addressing this issue ❤️

mtrojnar commented 8 months ago

@Niaxor Please include your operating system version and your command line for executing osslsigncode.

Niaxor commented 8 months ago

Windows 10 - Build 19045.3570

osslsigncode sign -pkcs11engine "<pkcs11_engine_path>" -pkcs11module "<pkcs11_module_path>" -pass "<passcode>" -key "<pkcs11_key_uri>" -certs "<path_to_cert>" -in "Input.exe" -out "Input.exe"

Just confirming I tested with Cygwin and Powershell and it is consistently reproducible. No graceful handling of the error, always results in a 0Kb input/output file, same error.

olszomal commented 8 months ago

You’re right, my mistake, I’m correcting it now. PR #315