search

mtrojnar / osslsigncode

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

bugfix: do not strip current signature in PE when sign/attach nested … #260

Closed yjh-styx closed 1 year ago

yjh-styx commented 1 year ago

…signature

mtrojnar commented 1 year ago

@olszomal Can you please review this PR?

olszomal commented 1 year ago

The current signature is stripped only to calculate a PE file's hash value. The PE file's hash does not contain the existing PKCS#7 signature.

While signing/attaching a nested signature, first we obtain a current signature from the previously signed file (cursig), then we create a new PKCS#7 signature (p7) with the calculated hash and finnaly we append the nested signature to the current signature.

I can't find any bug here. SignTool correctly validates such nested signatures: signtool verify /all /pa /v nested.exe

yjh-styx commented 1 year ago

See first condition in pe_pkcs7_get_file(): if (pe_ctx->siglen == 0 || pe_ctx->siglen > pe_ctx->fileend) { printf("Corrupted signature length: 0x%08X\n", pe_ctx->siglen); return NULL; / FAILED / } If PE is small (for example a dll with one small function) and first (presented) signature has large sertificates chain, siglen can be greather then sigpos and this condition (without patch) produce an error.

olszomal commented 1 year ago

Good point! I accept this pull request. Thank you very much for your patch. It's great that you contribute to the project.