Fix a heap overflow found by debuger, and make it harder to make that mistake again

Our public key functions assumed that they were always writing into a
large enough buffer.  In one case, they weren't.

(Incorporates fixes from sebastian)

9 files changed, 47 insertions, 26 deletions

diff --git a/src/or/config.c b/src/or/config.c
index 45f6114579..f8cfd29f84 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -5321,7 +5321,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\"", fname);
+    log_warn(LD_FS, "Unable to write state to file \"%s\"; "
+             "will try again later", fname);
     tor_free(fname);
     tor_free(contents);
     return -1;
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index 53e8a63ea3..7106294d54 100644
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -362,6 +362,7 @@ networkstatus_check_voter_signature(networkstatus_t *consensus,
   signed_digest = tor_malloc(signed_digest_len);
   if (crypto_pk_public_checksig(cert->signing_key,
                                 signed_digest,
+                                signed_digest_len,
                                 voter->signature,
                                 voter->signature_len) != DIGEST_LEN ||
       memcmp(signed_digest, consensus->networkstatus_digest, DIGEST_LEN)) {
diff --git a/src/or/onion.c b/src/or/onion.c
index 45c75b0717..bf72b4cab1 100644
--- a/src/or/onion.c
+++ b/src/or/onion.c
@@ -188,6 +188,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;
@@ -230,6 +231,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 edd24d8a3b..ab18d35298 100644
--- a/src/or/rendclient.c
+++ b/src/or/rendclient.c
@@ -193,6 +193,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/rendcommon.c b/src/or/rendcommon.c
index 1d96f3daf5..d6f5443815 100644
--- a/src/or/rendcommon.c
+++ b/src/or/rendcommon.c
@@ -700,7 +700,9 @@ rend_encode_service_descriptor(rend_service_descriptor_t *desc,
     cp += ipoint_len+1;
   }
   note_crypto_pk_op(REND_SERVER);
-  r = crypto_pk_private_sign_digest(key, cp, *str_out, cp-*str_out);
+  r = crypto_pk_private_sign_digest(key,
+                                    cp, buflen - (cp - *str_out),
+                                    *str_out, cp-*str_out);
   if (r<0) {
     tor_free(*str_out);
     return -1;
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index 9035ea4730..07f01aead9 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -979,7 +979,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.");
@@ -1424,7 +1425,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 c6c84a877d..7c8e36e402 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -4676,7 +4676,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 fc30c625bf..9ad84ed8db 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -571,10 +571,12 @@ router_append_dirobj_signature(char *buf, size_t buf_len, const char *digest,
                                crypto_pk_env_t *private_key)
 {
   char *signature;
-  size_t i;
+  size_t i, keysize;
 
-  signature = tor_malloc(crypto_pk_keysize(private_key));
-  if (crypto_pk_private_sign(private_key, signature, digest, DIGEST_LEN) < 0) {
+  keysize = crypto_pk_keysize(private_key);
+  signature = tor_malloc(keysize);
+  if (crypto_pk_private_sign(private_key, signature, keysize,
+                             digest, DIGEST_LEN) < 0) {
 
     log_warn(LD_BUG,"Couldn't sign digest.");
     goto err;
@@ -924,6 +926,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);
 
@@ -945,9 +948,10 @@ 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)
+  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);
diff --git a/src/or/test.c b/src/or/test.c
index 66fa5604be..103866b45a 100644
--- a/src/or/test.c
+++ b/src/or/test.c
@@ -701,25 +701,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(data2),
+                                        "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 */
@@ -734,19 +736,22 @@ 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(0, crypto_pk_public_checksig_digest(pk1, data1, 10, data2, 128));
-  test_eq(-1, crypto_pk_public_checksig_digest(pk1, data1, 11, 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(data1), 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 */
@@ -767,9 +772,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);
     }