few fixmes on cat manager

libclamav/asn1.c

@@ -747,6 +747,7 @@ static int asn1_parse_mscat(fmap_t *map, size_t offset, unsigned int size, crtmg
     unsigned int dsize, message_size, attrs_size;
     cli_crt_hashtype hashtype;
     SHA1Context ctx;
+    cli_crt *x509;
     int result;
 
     cli_dbgmsg("in asn1_parse_mscat\n");
@@ -835,22 +836,23 @@ static int asn1_parse_mscat(fmap_t *map, size_t offset, unsigned int size, crtmg
 	    if(dsize)
 		break;
 	    if(newcerts.crts) {
-		cli_crt *x509 = newcerts.crts;
+		x509 = newcerts.crts;
 		cli_dbgmsg("asn1_parse_mscat: %u new certificates collected\n", newcerts.items);
 		while(x509) {
 		    cli_crt *parent = crtmgr_verify_crt(cmgr, x509);
 		    if(parent) {
 			x509->codeSign &= parent->codeSign;
 			x509->timeSign &= parent->timeSign;
-			if(crtmgr_add(cmgr, x509)) {
-			    /* FIXME handle error */
-			}
+			if(crtmgr_add(cmgr, x509))
+			    break;
 			crtmgr_del(&newcerts, x509);
 			x509 = newcerts.crts;
 			continue;
 		    }
 		    x509 = x509->next;
 		}
+		if(x509)
+		    break;
 		if(newcerts.items)
 		    cli_dbgmsg("asn1_parse_mscat: %u certificates did not verify\n", newcerts.items);
 		crtmgr_free(&newcerts);
@@ -1020,7 +1022,7 @@ static int asn1_parse_mscat(fmap_t *map, size_t offset, unsigned int size, crtmg
 	    cli_dbgmsg("asn1_parse_mscat: failed to read encryptedDigest\n");
 	    break;
 	}
-	if(crtmgr_verify_pkcs7(cmgr, issuer, serial, asn1.content, asn1.size, CLI_SHA1RSA, sha1, VRFY_CODE)) {
+	if(!(x509 = crtmgr_verify_pkcs7(cmgr, issuer, serial, asn1.content, asn1.size, CLI_SHA1RSA, sha1, VRFY_CODE))) {
 	    cli_dbgmsg("asn1_parse_mscat: pkcs7 signature verification failed\n");
 	    break;
 	}
@@ -1193,6 +1195,10 @@ static int asn1_parse_mscat(fmap_t *map, size_t offset, unsigned int size, crtmg
 			deep.size = 1;
 		    else if(deep.size)
 			cli_dbgmsg("asn1_parse_mscat: extra data in countersignature signing-time\n");
+		    else if(sigdate < x509->not_before || sigdate > x509->not_after) {
+			cli_dbgmsg("asn1_parse_mscat: countersignature timestamp outside cert validity\n");
+			deep.size = 1;
+		    }
 		    break;
 		}
 	    }
@@ -1264,7 +1270,7 @@ static int asn1_parse_mscat(fmap_t *map, size_t offset, unsigned int size, crtmg
 	    cli_dbgmsg("asn1_parse_mscat: failed to read countersignature encryptedDigest\n");
 	    break;
 	}
-	if(crtmgr_verify_pkcs7(cmgr, issuer, serial, asn1.content, asn1.size, hashtype, sha1, VRFY_TIME)) {
+	if(!crtmgr_verify_pkcs7(cmgr, issuer, serial, asn1.content, asn1.size, hashtype, sha1, VRFY_TIME)) {
 	    cli_dbgmsg("asn1_parse_mscat: pkcs7 countersignature verification failed\n");
 	    break;
 	}
@@ -1279,7 +1285,6 @@ static int asn1_parse_mscat(fmap_t *map, size_t offset, unsigned int size, crtmg
 
 int asn1_load_mscat(fmap_t *map, struct cl_engine *engine) {
     struct cli_asn1 c;
-    char *virname;
     unsigned int size;
     struct cli_matcher *db;
     int i;
@@ -1406,11 +1411,6 @@ int asn1_load_mscat(fmap_t *map, struct cl_engine *engine) {
 		    sprintf(&sha1[i*2], "%02x", ((uint8_t *)(tagval3.content))[i]);
 		cli_dbgmsg("asn1_load_mscat: got hash %s (%s)\n", sha1, (hashtype == 2) ? "PE" : "CAB");
 	    }
-	    /* FIXME might as well use a static buf */
-	    virname = cli_mpool_virname(engine->mempool, "CAT", 1);
-	    if(!virname)
-		return 1;
-
 	    if(!engine->hm_fp) {
 		if(!(engine->hm_fp = mpool_calloc(engine->mempool, 1, sizeof(*db)))) {
 		    tag.size = 1;;
@@ -1420,10 +1420,8 @@ int asn1_load_mscat(fmap_t *map, struct cl_engine *engine) {
 		engine->hm_fp->mempool = engine->mempool;
 #endif
 	    }
-
-	    if(hm_addhash_bin(engine->hm_fp, tagval3.content, CLI_HASH_SHA1, hashtype, virname)) {
+	    if(hm_addhash_bin(engine->hm_fp, tagval3.content, CLI_HASH_SHA1, hashtype, NULL)) {
 		cli_warnmsg("asn1_load_mscat: failed to add hash\n");
-		mpool_free(engine->mempool, (void *)virname);
 		return 1;
 	    }
 	}

libclamav/crtmgr.c

@@ -87,6 +87,7 @@ int crtmgr_add(crtmgr *m, cli_crt *x509) {
 	    i->certSign |= x509->certSign;
 	    i->codeSign |= x509->codeSign;
 	    i->timeSign |= x509->timeSign;
+	    return 0;
 	}
     }
 
@@ -260,39 +261,45 @@ static int crtmgr_rsa_verify(cli_crt *x509, mp_int *sig, cli_crt_hashtype hashty
 
 
 cli_crt *crtmgr_verify_crt(crtmgr *m, cli_crt *x509) {
-    cli_crt *i = m->crts;
+    cli_crt *i = m->crts, *best = NULL;
+    int score = 0;
 
     for(i = m->crts; i; i = i->next) {
 	if(i->certSign &&
-	   (x509->codeSign & i->codeSign) == x509->codeSign &&
-	   (x509->timeSign & i->timeSign) == x509->timeSign &&
 	   !memcmp(i->subject, x509->issuer, sizeof(i->subject)) &&
-	   !crtmgr_rsa_verify(i, &x509->sig, x509->hashtype, x509->tbshash))
+	   !crtmgr_rsa_verify(i, &x509->sig, x509->hashtype, x509->tbshash)) {
+	    int curscore;
+	    if((x509->codeSign & i->codeSign) == x509->codeSign && (x509->timeSign & i->timeSign) == x509->timeSign)
 		return i;
+	    curscore = (x509->codeSign & i->codeSign) + (x509->timeSign & i->timeSign);
+	    if(curscore > score) {
+		best = i;
+		score = curscore;
 	    }
-    return NULL;
+	}
+    }
+    return best;
 }
 
-int crtmgr_verify_pkcs7(crtmgr *m, const uint8_t *issuer, const uint8_t *serial, const void *signature, unsigned int signature_len, cli_crt_hashtype hashtype, const uint8_t *refhash, cli_vrfy_type vrfytype) {
+cli_crt *crtmgr_verify_pkcs7(crtmgr *m, const uint8_t *issuer, const uint8_t *serial, const void *signature, unsigned int signature_len, cli_crt_hashtype hashtype, const uint8_t *refhash, cli_vrfy_type vrfytype) {
     cli_crt *i;
     mp_int sig;
     int ret;
 
     if(signature_len < 1024/8 || signature_len > 4096/8+1) {
 	cli_dbgmsg("crtmgr_verify_pkcs7: unsupported sig len: %u\n", signature_len);
-       return 1;
+	return NULL;
     }
     if((ret = mp_init(&sig))) {
 	cli_errmsg("crtmgr_verify_pkcs7: mp_init failed with %d\n", ret);
-	return 1;
+	return NULL;
     }
 
     if((ret=mp_read_unsigned_bin(&sig, signature, signature_len))) {
 	cli_warnmsg("crtmgr_verify_pkcs7: mp_read_unsigned_bin failed with %d\n", ret);
-	return 1;
+	return NULL;
     }
 
-    ret = 1;
     for(i = m->crts; i; i = i->next) {
 	if(vrfytype == VRFY_CODE && !i->codeSign)
 	    continue;
@@ -300,13 +307,11 @@ int crtmgr_verify_pkcs7(crtmgr *m, const uint8_t *issuer, const uint8_t *serial,
 	    continue;
 	if(!memcmp(i->issuer, issuer, sizeof(i->issuer)) &&
 	   !memcmp(i->serial, serial, sizeof(i->serial)) &&
-	   !crtmgr_rsa_verify(i, &sig, hashtype, refhash)) {
-	    ret = 0;
+	   !crtmgr_rsa_verify(i, &sig, hashtype, refhash))
 	    break;
     }
-    }
     mp_clear(&sig);
-    return ret;
+    return i;
 }
 
 /* DC=com, DC=microsoft, CN=Microsoft Root Certificate Authority */

libclamav/crtmgr.h

@@ -61,7 +61,7 @@ int crtmgr_add(crtmgr *m, cli_crt *x509);
 cli_crt *crtmgr_lookup(crtmgr *m, cli_crt *x509);
 void crtmgr_del(crtmgr *m, cli_crt *x509);
 cli_crt *crtmgr_verify_crt(crtmgr *m, cli_crt *x509);
-int crtmgr_verify_pkcs7(crtmgr *m, const uint8_t *issuer, const uint8_t *serial, const void *signature, unsigned int signature_len, cli_crt_hashtype hashtype, const uint8_t *refhash, cli_vrfy_type vrfytype);
+cli_crt *crtmgr_verify_pkcs7(crtmgr *m, const uint8_t *issuer, const uint8_t *serial, const void *signature, unsigned int signature_len, cli_crt_hashtype hashtype, const uint8_t *refhash, cli_vrfy_type vrfytype);
 int crtmgr_add_roots(crtmgr *m);