diff --git a/changes/bug2332 b/changes/bug2332 new file mode 100644 index 0000000000..5f73ddd7af --- /dev/null +++ b/changes/bug2332 @@ -0,0 +1,4 @@ + o Minor bugfixes + - Fix a bug with handling misformed replies to reverse DNS lookup + requests in DNSPort. Bugfix on Tor 0.2.0.1-alpha. Related to a bug + reported by doorss. diff --git a/changes/tolen_asserts b/changes/tolen_asserts new file mode 100644 index 0000000000..a9834ab669 --- /dev/null +++ b/changes/tolen_asserts @@ -0,0 +1,8 @@ + o Major bugfixes (security) + - Fix a heap overflow bug where an adversary could cause heap + corruption. This bug potentially allows remote code execution + attacks. Found by debuger. Fixes CVE-2011-0427. Bugfix on + 0.1.2.10-rc. + o Defensive programming + - Introduce output size checks on all of our decryption functions. + diff --git a/src/common/crypto.c b/src/common/crypto.c index e47fa5602c..15b58188ed 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -807,9 +807,12 @@ crypto_pk_copy_full(crypto_pk_env_t *env) * in env, using the padding method padding. On success, * write the result to to, and return the number of bytes * written. On failure, return -1. + * + * tolen is the number of writable bytes in to, and must be + * at least the length of the modulus of env. */ int -crypto_pk_public_encrypt(crypto_pk_env_t *env, char *to, +crypto_pk_public_encrypt(crypto_pk_env_t *env, char *to, size_t tolen, const char *from, size_t fromlen, int padding) { int r; @@ -817,6 +820,7 @@ crypto_pk_public_encrypt(crypto_pk_env_t *env, char *to, tor_assert(from); tor_assert(to); tor_assert(fromlen= crypto_pk_keysize(env)); r = RSA_public_encrypt((int)fromlen, (unsigned char*)from, (unsigned char*)to, @@ -832,9 +836,13 @@ crypto_pk_public_encrypt(crypto_pk_env_t *env, char *to, * in env, using the padding method padding. On success, * write the result to to, and return the number of bytes * written. On failure, return -1. + * + * tolen is the number of writable bytes in to, and must be + * at least the length of the modulus of env. */ int crypto_pk_private_decrypt(crypto_pk_env_t *env, char *to, + size_t tolen, const char *from, size_t fromlen, int padding, int warnOnFailure) { @@ -844,6 +852,7 @@ crypto_pk_private_decrypt(crypto_pk_env_t *env, char *to, tor_assert(to); tor_assert(env->key); tor_assert(fromlen= crypto_pk_keysize(env)); if (!env->key->p) /* Not a private key */ return -1; @@ -864,9 +873,13 @@ crypto_pk_private_decrypt(crypto_pk_env_t *env, char *to, * public key in env, using PKCS1 padding. On success, write the * signed data to to, and return the number of bytes written. * On failure, return -1. + * + * tolen is the number of writable bytes in to, and must be + * at least the length of the modulus of env. */ int crypto_pk_public_checksig(crypto_pk_env_t *env, char *to, + size_t tolen, const char *from, size_t fromlen) { int r; @@ -874,6 +887,7 @@ crypto_pk_public_checksig(crypto_pk_env_t *env, char *to, tor_assert(from); tor_assert(to); tor_assert(fromlen < INT_MAX); + tor_assert(tolen >= crypto_pk_keysize(env)); r = RSA_public_decrypt((int)fromlen, (unsigned char*)from, (unsigned char*)to, env->key, RSA_PKCS1_PADDING); @@ -896,6 +910,7 @@ crypto_pk_public_checksig_digest(crypto_pk_env_t *env, const char *data, { char digest[DIGEST_LEN]; char *buf; + size_t buflen; int r; tor_assert(env); @@ -908,8 +923,9 @@ crypto_pk_public_checksig_digest(crypto_pk_env_t *env, const char *data, log_warn(LD_BUG, "couldn't compute digest"); return -1; } - buf = tor_malloc(crypto_pk_keysize(env)+1); - r = crypto_pk_public_checksig(env,buf,sig,siglen); + buflen = crypto_pk_keysize(env)+1; + buf = tor_malloc(buflen); + r = crypto_pk_public_checksig(env,buf,buflen,sig,siglen); if (r != DIGEST_LEN) { log_warn(LD_CRYPTO, "Invalid signature"); tor_free(buf); @@ -929,9 +945,12 @@ crypto_pk_public_checksig_digest(crypto_pk_env_t *env, const char *data, * env, using PKCS1 padding. On success, write the signature to * to, and return the number of bytes written. On failure, return * -1. + * + * tolen is the number of writable bytes in to, and must be + * at least the length of the modulus of env. */ int -crypto_pk_private_sign(crypto_pk_env_t *env, char *to, +crypto_pk_private_sign(crypto_pk_env_t *env, char *to, size_t tolen, const char *from, size_t fromlen) { int r; @@ -939,6 +958,7 @@ crypto_pk_private_sign(crypto_pk_env_t *env, char *to, tor_assert(from); tor_assert(to); tor_assert(fromlen < INT_MAX); + tor_assert(tolen >= crypto_pk_keysize(env)); if (!env->key->p) /* Not a private key */ return -1; @@ -957,16 +977,19 @@ crypto_pk_private_sign(crypto_pk_env_t *env, char *to, * from; sign the data with the private key in env, and * store it in to. Return the number of bytes written on * success, and -1 on failure. + * + * tolen is the number of writable bytes in to, and must be + * at least the length of the modulus of env. */ int -crypto_pk_private_sign_digest(crypto_pk_env_t *env, char *to, +crypto_pk_private_sign_digest(crypto_pk_env_t *env, char *to, size_t tolen, const char *from, size_t fromlen) { int r; char digest[DIGEST_LEN]; if (crypto_digest(digest,from,fromlen)<0) return -1; - r = crypto_pk_private_sign(env,to,digest,DIGEST_LEN); + r = crypto_pk_private_sign(env,to,tolen,digest,DIGEST_LEN); memset(digest, 0, sizeof(digest)); return r; } @@ -990,7 +1013,7 @@ crypto_pk_private_sign_digest(crypto_pk_env_t *env, char *to, */ int crypto_pk_public_hybrid_encrypt(crypto_pk_env_t *env, - char *to, + char *to, size_t tolen, const char *from, size_t fromlen, int padding, int force) @@ -1013,8 +1036,13 @@ crypto_pk_public_hybrid_encrypt(crypto_pk_env_t *env, if (!force && fromlen+overhead <= pkeylen) { /* It all fits in a single encrypt. */ - return crypto_pk_public_encrypt(env,to,from,fromlen,padding); + return crypto_pk_public_encrypt(env,to, + tolen, + from,fromlen,padding); } + tor_assert(tolen >= fromlen + overhead + CIPHER_KEY_LEN); + tor_assert(tolen >= pkeylen); + cipher = crypto_new_cipher_env(); if (!cipher) return -1; if (crypto_cipher_generate_key(cipher)<0) @@ -1036,7 +1064,7 @@ crypto_pk_public_hybrid_encrypt(crypto_pk_env_t *env, /* Length of symmetrically encrypted data. */ symlen = fromlen-(pkeylen-overhead-CIPHER_KEY_LEN); - outlen = crypto_pk_public_encrypt(env,to,buf,pkeylen-overhead,padding); + outlen = crypto_pk_public_encrypt(env,to,tolen,buf,pkeylen-overhead,padding); if (outlen!=(int)pkeylen) { goto err; } @@ -1062,6 +1090,7 @@ crypto_pk_public_hybrid_encrypt(crypto_pk_env_t *env, int crypto_pk_private_hybrid_decrypt(crypto_pk_env_t *env, char *to, + size_t tolen, const char *from, size_t fromlen, int padding, int warnOnFailure) @@ -1075,11 +1104,12 @@ crypto_pk_private_hybrid_decrypt(crypto_pk_env_t *env, pkeylen = crypto_pk_keysize(env); if (fromlen <= pkeylen) { - return crypto_pk_private_decrypt(env,to,from,fromlen,padding, + return crypto_pk_private_decrypt(env,to,tolen,from,fromlen,padding, warnOnFailure); } + buf = tor_malloc(pkeylen+1); - outlen = crypto_pk_private_decrypt(env,buf,from,pkeylen,padding, + outlen = crypto_pk_private_decrypt(env,buf,pkeylen+1,from,pkeylen,padding, warnOnFailure); if (outlen<0) { log_fn(warnOnFailure?LOG_WARN:LOG_DEBUG, LD_CRYPTO, @@ -1097,6 +1127,7 @@ crypto_pk_private_hybrid_decrypt(crypto_pk_env_t *env, } memcpy(to,buf+CIPHER_KEY_LEN,outlen-CIPHER_KEY_LEN); outlen -= CIPHER_KEY_LEN; + tor_assert(tolen - outlen >= fromlen - pkeylen); r = crypto_cipher_decrypt(cipher, to+outlen, from+pkeylen, fromlen-pkeylen); if (r<0) goto err; diff --git a/src/common/crypto.h b/src/common/crypto.h index 0801728281..29ba36cdf6 100644 --- a/src/common/crypto.h +++ b/src/common/crypto.h @@ -123,23 +123,25 @@ crypto_pk_env_t *crypto_pk_dup_key(crypto_pk_env_t *orig); crypto_pk_env_t *crypto_pk_copy_full(crypto_pk_env_t *orig); int crypto_pk_key_is_private(const crypto_pk_env_t *key); -int crypto_pk_public_encrypt(crypto_pk_env_t *env, char *to, +int crypto_pk_public_encrypt(crypto_pk_env_t *env, char *to, size_t tolen, const char *from, size_t fromlen, int padding); -int crypto_pk_private_decrypt(crypto_pk_env_t *env, char *to, +int crypto_pk_private_decrypt(crypto_pk_env_t *env, char *to, size_t tolen, const char *from, size_t fromlen, int padding, int warnOnFailure); -int crypto_pk_public_checksig(crypto_pk_env_t *env, char *to, +int crypto_pk_public_checksig(crypto_pk_env_t *env, char *to, size_t tolen, const char *from, size_t fromlen); int crypto_pk_public_checksig_digest(crypto_pk_env_t *env, const char *data, size_t datalen, const char *sig, size_t siglen); -int crypto_pk_private_sign(crypto_pk_env_t *env, char *to, +int crypto_pk_private_sign(crypto_pk_env_t *env, char *to, size_t tolen, const char *from, size_t fromlen); -int crypto_pk_private_sign_digest(crypto_pk_env_t *env, char *to, +int crypto_pk_private_sign_digest(crypto_pk_env_t *env, char *to, size_t tolen, const char *from, size_t fromlen); int crypto_pk_public_hybrid_encrypt(crypto_pk_env_t *env, char *to, + size_t tolen, const char *from, size_t fromlen, int padding, int force); int crypto_pk_private_hybrid_decrypt(crypto_pk_env_t *env, char *to, + size_t tolen, const char *from, size_t fromlen, int padding, int warnOnFailure); diff --git a/src/or/config.c b/src/or/config.c index a27fd22b24..b124db1899 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -5182,8 +5182,8 @@ or_state_save(time_t now) tor_free(state); fname = get_datadir_fname("state"); if (write_str_to_file(fname, contents, 0)<0) { - log_warn(LD_FS, "Unable to write state to file \"%s\"; will try later", - fname); + log_warn(LD_FS, "Unable to write state to file \"%s\"; " + "will try again later", fname); global_state->LastWritten = -1; tor_free(fname); tor_free(contents); diff --git a/src/or/dnsserv.c b/src/or/dnsserv.c index c491656f73..d5faffb2c3 100644 --- a/src/or/dnsserv.c +++ b/src/or/dnsserv.c @@ -286,7 +286,7 @@ dnsserv_resolved(edge_connection_t *conn, char *ans = tor_strndup(answer, answer_len); evdns_server_request_add_ptr_reply(req, NULL, name, - (char*)answer, ttl); + ans, ttl); tor_free(ans); } else if (answer_type == RESOLVED_TYPE_ERROR) { err = DNS_ERR_NOTEXIST; diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 94bcb41002..dfc3a45f76 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -439,6 +439,7 @@ networkstatus_check_document_signature(const networkstatus_t *consensus, signed_digest = tor_malloc(signed_digest_len); if (crypto_pk_public_checksig(cert->signing_key, signed_digest, + signed_digest_len, sig->signature, sig->signature_len) < dlen || memcmp(signed_digest, consensus->digests.d[sig->alg], dlen)) { diff --git a/src/or/onion.c b/src/or/onion.c index 9db9145c78..323e0003e6 100644 --- a/src/or/onion.c +++ b/src/or/onion.c @@ -199,6 +199,7 @@ onion_skin_create(crypto_pk_env_t *dest_router_key, /* set meeting point, meeting cookie, etc here. Leave zero for now. */ if (crypto_pk_public_hybrid_encrypt(dest_router_key, onion_skin_out, + ONIONSKIN_CHALLENGE_LEN, challenge, DH_KEY_LEN, PK_PKCS1_OAEP_PADDING, 1)<0) goto err; @@ -241,6 +242,7 @@ onion_skin_server_handshake(const char *onion_skin, /*ONIONSKIN_CHALLENGE_LEN*/ break; note_crypto_pk_op(DEC_ONIONSKIN); len = crypto_pk_private_hybrid_decrypt(k, challenge, + ONIONSKIN_CHALLENGE_LEN, onion_skin, ONIONSKIN_CHALLENGE_LEN, PK_PKCS1_OAEP_PADDING,0); if (len>0) diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 7c626c6a64..b8526b676d 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -184,6 +184,7 @@ rend_client_send_introduction(origin_circuit_t *introcirc, /*XXX maybe give crypto_pk_public_hybrid_encrypt a max_len arg, * to avoid buffer overflows? */ r = crypto_pk_public_hybrid_encrypt(intro_key, payload+DIGEST_LEN, + sizeof(payload)-DIGEST_LEN, tmp, (int)(dh_offset+DH_KEY_LEN), PK_PKCS1_OAEP_PADDING, 0); diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 44b5a4b4c0..1d64cf41e3 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -928,7 +928,8 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request, /* Next N bytes is encrypted with service key */ note_crypto_pk_op(REND_SERVER); r = crypto_pk_private_hybrid_decrypt( - intro_key,buf,(char*)(request+DIGEST_LEN),request_len-DIGEST_LEN, + intro_key,buf,sizeof(buf), + (char*)(request+DIGEST_LEN),request_len-DIGEST_LEN, PK_PKCS1_OAEP_PADDING,1); if (r<0) { log_warn(LD_PROTOCOL, "Couldn't decrypt INTRODUCE2 cell."); @@ -1365,7 +1366,8 @@ rend_service_intro_has_opened(origin_circuit_t *circuit) goto err; len += 20; note_crypto_pk_op(REND_SERVER); - r = crypto_pk_private_sign_digest(intro_key, buf+len, buf, len); + r = crypto_pk_private_sign_digest(intro_key, buf+len, sizeof(buf)-len, + buf, len); if (r<0) { log_warn(LD_BUG, "Internal error: couldn't sign introduction request."); reason = END_CIRC_REASON_INTERNAL; diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 253b787217..e29b4c49d8 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -5016,7 +5016,8 @@ routerinfo_incompatible_with_extrainfo(routerinfo_t *ri, extrainfo_t *ei, if (ei->pending_sig) { char signed_digest[128]; - if (crypto_pk_public_checksig(ri->identity_pkey, signed_digest, + if (crypto_pk_public_checksig(ri->identity_pkey, + signed_digest, sizeof(signed_digest), ei->pending_sig, ei->pending_sig_len) != DIGEST_LEN || memcmp(signed_digest, ei->cache_info.signed_descriptor_digest, DIGEST_LEN)) { diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 691b9beabc..66d024ecd4 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -702,11 +702,13 @@ router_append_dirobj_signature(char *buf, size_t buf_len, const char *digest, size_t digest_len, crypto_pk_env_t *private_key) { char *signature; - size_t i; + size_t i, keysize; int siglen; - signature = tor_malloc(crypto_pk_keysize(private_key)); - siglen = crypto_pk_private_sign(private_key, signature, digest, digest_len); + keysize = crypto_pk_keysize(private_key); + signature = tor_malloc(keysize); + siglen = crypto_pk_private_sign(private_key, signature, keysize, + digest, digest_len); if (siglen < 0) { log_warn(LD_BUG,"Couldn't sign digest."); goto err; @@ -1059,6 +1061,7 @@ check_signature_token(const char *digest, const char *doctype) { char *signed_digest; + size_t keysize; const int check_authority = (flags & CST_CHECK_AUTHORITY); const int check_objtype = ! (flags & CST_NO_CHECK_OBJTYPE); @@ -1080,10 +1083,11 @@ check_signature_token(const char *digest, } } - signed_digest = tor_malloc(tok->object_size); - if (crypto_pk_public_checksig(pkey, signed_digest, tok->object_body, - tok->object_size) - < digest_len) { + keysize = crypto_pk_keysize(pkey); + signed_digest = tor_malloc(keysize); + if (crypto_pk_public_checksig(pkey, signed_digest, keysize, + tok->object_body, tok->object_size) + < DIGEST_LEN) { log_warn(LD_DIR, "Error reading %s: invalid signature.", doctype); tor_free(signed_digest); return -1; diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index 4638786189..8093c9c898 100644 --- a/src/test/test_crypto.c +++ b/src/test/test_crypto.c @@ -343,25 +343,27 @@ test_crypto_pk(void) test_eq(128, crypto_pk_keysize(pk1)); test_eq(128, crypto_pk_keysize(pk2)); - test_eq(128, crypto_pk_public_encrypt(pk2, data1, "Hello whirled.", 15, + test_eq(128, crypto_pk_public_encrypt(pk2, data1, sizeof(data1), + "Hello whirled.", 15, PK_PKCS1_OAEP_PADDING)); - test_eq(128, crypto_pk_public_encrypt(pk1, data2, "Hello whirled.", 15, + test_eq(128, crypto_pk_public_encrypt(pk1, data2, sizeof(data1), + "Hello whirled.", 15, PK_PKCS1_OAEP_PADDING)); /* oaep padding should make encryption not match */ test_memneq(data1, data2, 128); - test_eq(15, crypto_pk_private_decrypt(pk1, data3, data1, 128, + test_eq(15, crypto_pk_private_decrypt(pk1, data3, sizeof(data3), data1, 128, PK_PKCS1_OAEP_PADDING,1)); test_streq(data3, "Hello whirled."); memset(data3, 0, 1024); - test_eq(15, crypto_pk_private_decrypt(pk1, data3, data2, 128, + test_eq(15, crypto_pk_private_decrypt(pk1, data3, sizeof(data3), data2, 128, PK_PKCS1_OAEP_PADDING,1)); test_streq(data3, "Hello whirled."); /* Can't decrypt with public key. */ - test_eq(-1, crypto_pk_private_decrypt(pk2, data3, data2, 128, + test_eq(-1, crypto_pk_private_decrypt(pk2, data3, sizeof(data3), data2, 128, PK_PKCS1_OAEP_PADDING,1)); /* Try again with bad padding */ memcpy(data2+1, "XYZZY", 5); /* This has fails ~ once-in-2^40 */ - test_eq(-1, crypto_pk_private_decrypt(pk1, data3, data2, 128, + test_eq(-1, crypto_pk_private_decrypt(pk1, data3, sizeof(data3), data2, 128, PK_PKCS1_OAEP_PADDING,1)); /* File operations: save and load private key */ @@ -376,19 +378,21 @@ test_crypto_pk(void) get_fname("xyzzy")) < 0); test_assert(! crypto_pk_read_private_key_from_filename(pk2, get_fname("pkey1"))); - test_eq(15, crypto_pk_private_decrypt(pk2, data3, data1, 128, + test_eq(15, crypto_pk_private_decrypt(pk2, data3, sizeof(data3), data1, 128, PK_PKCS1_OAEP_PADDING,1)); /* Now try signing. */ strlcpy(data1, "Ossifrage", 1024); - test_eq(128, crypto_pk_private_sign(pk1, data2, data1, 10)); - test_eq(10, crypto_pk_public_checksig(pk1, data3, data2, 128)); + test_eq(128, crypto_pk_private_sign(pk1, data2, sizeof(data2), data1, 10)); + test_eq(10, crypto_pk_public_checksig(pk1, data3, sizeof(data3), data2, 128)); test_streq(data3, "Ossifrage"); /* Try signing digests. */ - test_eq(128, crypto_pk_private_sign_digest(pk1, data2, data1, 10)); - test_eq(20, crypto_pk_public_checksig(pk1, data3, data2, 128)); + test_eq(128, crypto_pk_private_sign_digest(pk1, data2, sizeof(data2), + data1, 10)); + test_eq(20, crypto_pk_public_checksig(pk1, data3, sizeof(data3), data2, 128)); test_eq(0, crypto_pk_public_checksig_digest(pk1, data1, 10, data2, 128)); test_eq(-1, crypto_pk_public_checksig_digest(pk1, data1, 11, data2, 128)); + /*XXXX test failed signing*/ /* Try encoding */ @@ -409,9 +413,11 @@ test_crypto_pk(void) continue; p = (i==0)?PK_NO_PADDING: (i==1)?PK_PKCS1_PADDING:PK_PKCS1_OAEP_PADDING; - len = crypto_pk_public_hybrid_encrypt(pk1,data2,data1,j,p,0); + len = crypto_pk_public_hybrid_encrypt(pk1,data2,sizeof(data2), + data1,j,p,0); test_assert(len>=0); - len = crypto_pk_private_hybrid_decrypt(pk1,data3,data2,len,p,1); + len = crypto_pk_private_hybrid_decrypt(pk1,data3,sizeof(data3), + data2,len,p,1); test_eq(len,j); test_memeq(data1,data3,j); }