From b8795fde95c9807f5d65c84fbe411dbfce7534a1 Mon Sep 17 00:00:00 2001 From: Micah Snyder Date: Tue, 3 Dec 2024 17:17:27 -0500 Subject: [PATCH 1/2] Fix CVD dsig verification when hash starts with zeros Occasionally the MD5 hash for RSA-based digital signature verification begins with zeros. A bug in how we convert the RSA decoded plain text from a big number back to a hex string causes it to write the number to the far left of the plain text buffer. If the number is smaller than a hash, then zero-padding ends up on the right when it should've been on the left. Additional fix: BN_bn2bin() will write zero bytes if the bignum is 0. So there is no point "error checking" the BN_bn2bin() call. Thanks to Tom Judge for noticing these shenanigans. Ref: https://github.com/openssl/openssl/issues/2101 Side note: BN_num_bytes() will also return 0 if the bignum is 0, which is fine. --- libclamav/dsig.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/libclamav/dsig.c b/libclamav/dsig.c index b0cf212f07..7feb0a6b16 100644 --- a/libclamav/dsig.c +++ b/libclamav/dsig.c @@ -88,7 +88,7 @@ static unsigned char *cli_decodesig(const char *sig, unsigned int plen, BIGNUM * BIGNUM *r = NULL, *p = NULL, *c = NULL; BN_CTX *bn_ctx = NULL; unsigned int bn_bytes; - ; + unsigned char *plain_offset = NULL; r = BN_new(); if (!r) { @@ -144,9 +144,12 @@ static unsigned char *cli_decodesig(const char *sig, unsigned int plen, BIGNUM * cli_errmsg("cli_decodesig: Can't allocate memory for 'plain'\n"); goto done; } - if (!BN_bn2bin(p, plain)) { - goto done; - } + + // If bn_bytes is smaller than plen, we need to offset the plain buffer. + // If we didn't, then a hash that should start with 00 would end with 00 instead. + plain_offset = plain + plen - bn_bytes; + + BN_bn2bin(p, plain_offset); ret_sig = plain; plain = NULL; From 4d389f3eedb8f2aa18ab90755d27abfbd7b00810 Mon Sep 17 00:00:00 2001 From: Micah Snyder Date: Mon, 16 Dec 2024 17:49:52 -0500 Subject: [PATCH 2/2] Sigtool: CVD build hack for verify bug in 1.1 -> 1.4 Have --build retry if the tar.gz MD5 starts with 00. --- sigtool/sigtool.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/sigtool/sigtool.c b/sigtool/sigtool.c index 8e36a4684b..33d37b3866 100644 --- a/sigtool/sigtool.c +++ b/sigtool/sigtool.c @@ -1155,6 +1155,20 @@ static int build(const struct optstruct *opts) free(tarfile); return -1; } + + // Check if the MD5 starts with 00. If it does, we'll return CL_ELAST_ERROR. The caller may try again for better luck. + // This is to avoid a bug in hash verification with ClamAV 1.1 -> 1.4. The bug was fixed in 1.5.0. + // TODO: Remove this workaround when no one is using those versions. + if (pt[0] == '0' && pt[1] == '0') { + // print out the pt hash + mprintf(LOGG_INFO, "The tar.gz MD5 starts with 00, which will fail to verify in ClamAV 1.1 -> 1.4: %s\n", pt); + fclose(fh); + unlink(tarfile); + free(tarfile); + free(pt); + return CL_ELAST_ERROR; + } + rewind(fh); sprintf(header + strlen(header), "%s:", pt); free(pt); @@ -3768,9 +3782,15 @@ int main(int argc, char **argv) ret = asciinorm(opts); else if (optget(opts, "utf16-decode")->enabled) ret = utf16decode(opts); - else if (optget(opts, "build")->enabled) + else if (optget(opts, "build")->enabled) { ret = build(opts); - else if (optget(opts, "unpack")->enabled) + if (ret == CL_ELAST_ERROR) { + // build() returns CL_ELAST_ERROR the hash starts with 00. This will fail to verify with ClamAV 1.1 -> 1.4. + // Retry the build again to get new hashes. + mprintf(LOGG_WARNING, "Retrying the build for a chance at a better hash.\n"); + ret = build(opts); + } + } else if (optget(opts, "unpack")->enabled) ret = unpack(opts); else if (optget(opts, "unpack-current")->enabled) ret = unpack(opts);