Add logic in routerparse to not read overlong private keys

I am not at all sure that it is possible to trigger a bug here,
but better safe than sorry.

diff --git a/src/common/crypto.c b/src/common/crypto.c
index 334398033136300a62198987997a47775a1a3650..da4e03edd8c3899dbd083ca0909f15fc43d56ae6 100644 (file)
--- a/src/common/crypto.c
+++ b/src/common/crypto.c
@@ -439,21 +439,23 @@ crypto_pk_generate_key(crypto_pk_env_t *env)
   return 0;
 }
 
-/** Read a PEM-encoded private key from the string <b>s</b> into <b>env</b>.
- * Return 0 on success, -1 on failure.
+/** Read a PEM-encoded private key from the <b>len</b>-byte string <b>s</b>
+ * into <b>env</b>.  Return 0 on success, -1 on failure.  If len is -1,
+ * the string is nul-terminated.
  */
 /* Used here, and used for testing. */
 int
 crypto_pk_read_private_key_from_string(crypto_pk_env_t *env,
-                                       const char *s)
+                                       const char *s, ssize_t len)
 {
   BIO *b;
 
   tor_assert(env);
   tor_assert(s);
+  tor_assert(len < INT_MAX && len < SIZE_T_CEILING);
 
-  /* Create a read-only memory BIO, backed by the NUL-terminated string 's' */
-  b = BIO_new_mem_buf((char*)s, -1);
+  /* Create a read-only memory BIO, backed by the string 's' */
+  b = BIO_new_mem_buf((char*)s, (int)len);
 
   if (env->key)
     RSA_free(env->key);
@@ -487,7 +489,7 @@ crypto_pk_read_private_key_from_filename(crypto_pk_env_t *env,
   }
 
   /* Try to parse it. */
-  r = crypto_pk_read_private_key_from_string(env, contents);
+  r = crypto_pk_read_private_key_from_string(env, contents, -1);
   tor_free(contents);
   if (r)
     return -1; /* read_private_key_from_string already warned, so we don't.*/

diff --git a/src/common/crypto.h b/src/common/crypto.h
index 4fb06be41d09759f56610bbec91efa8dd0e1649f..713a988278297fef15f106753f482a3289e77f27 100644 (file)
--- a/src/common/crypto.h
+++ b/src/common/crypto.h
@@ -82,7 +82,7 @@ int crypto_pk_write_private_key_to_string(crypto_pk_env_t *env,
 int crypto_pk_read_public_key_from_string(crypto_pk_env_t *env,
                                           const char *src, size_t len);
 int crypto_pk_read_private_key_from_string(crypto_pk_env_t *env,
-                                           const char *s);
+                                           const char *s, ssize_t len);
 int crypto_pk_write_private_key_to_filename(crypto_pk_env_t *env,
                                             const char *fname);
 

diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index fc30c625bf8c985d006b60ed05c9fa46929d9cbc..6ca22933755a5718d7cd53d2d32e158c4989ae91 100644 (file)
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -3132,7 +3132,7 @@ get_next_token(memarea_t *area,
       RET_ERR("Couldn't parse public key.");
   } else if (!strcmp(tok->object_type, "RSA PRIVATE KEY")) { /* private key */
     tok->key = crypto_new_pk_env();
-    if (crypto_pk_read_private_key_from_string(tok->key, obstart))
+    if (crypto_pk_read_private_key_from_string(tok->key, obstart, eol-obstart))
       RET_ERR("Couldn't parse private key.");
   } else { /* If it's something else, try to base64-decode it */
     int r;

diff --git a/src/or/test.c b/src/or/test.c
index 66fa5604bee467c1c735aa8b1c9d319ad7356891..c4458d833d396c0b80179f1ed2a91cad716a3942 100644 (file)
--- a/src/or/test.c
+++ b/src/or/test.c
@@ -3361,11 +3361,11 @@ test_v3_networkstatus(void)
   sign_skey_leg1 = pk_generate(4);
 
   test_assert(!crypto_pk_read_private_key_from_string(sign_skey_1,
-                                                      AUTHORITY_SIGNKEY_1));
+                                                      AUTHORITY_SIGNKEY_1,-1));
   test_assert(!crypto_pk_read_private_key_from_string(sign_skey_2,
-                                                      AUTHORITY_SIGNKEY_2));
+                                                      AUTHORITY_SIGNKEY_2,-1));
   test_assert(!crypto_pk_read_private_key_from_string(sign_skey_3,
-                                                      AUTHORITY_SIGNKEY_3));
+                                                      AUTHORITY_SIGNKEY_3,-1));
 
   test_assert(!crypto_pk_cmp_keys(sign_skey_1, cert1->signing_key));
   test_assert(!crypto_pk_cmp_keys(sign_skey_2, cert2->signing_key));