Followup: Make authority_cert_parse_from_string() take length too

9 files changed, 63 insertions, 26 deletions

diff --git a/src/feature/nodelist/routerlist.c b/src/feature/nodelist/routerlist.c
index bcc5c1f074..c3c72e9c78 100644
--- a/src/feature/nodelist/routerlist.c
+++ b/src/feature/nodelist/routerlist.c
@@ -555,7 +555,8 @@ trusted_dirs_load_certs_from_string(const char *contents, int source,
   int added_trusted_cert = 0;
 
   for (s = contents; *s; s = eos) {
-    authority_cert_t *cert = authority_cert_parse_from_string(s, &eos);
+    authority_cert_t *cert = authority_cert_parse_from_string(s, strlen(s),
+                                                              &eos);
     cert_list_t *cl;
     if (!cert) {
       failure_code = -1;
diff --git a/src/feature/nodelist/routerparse.c b/src/feature/nodelist/routerparse.c
index 9abdfb614c..8a5efc007e 100644
--- a/src/feature/nodelist/routerparse.c
+++ b/src/feature/nodelist/routerparse.c
@@ -2308,7 +2308,8 @@ extrainfo_parse_entry_from_string(const char *s, const char *end,
 /** Parse a key certificate from <b>s</b>; point <b>end-of-string</b> to
  * the first character after the certificate. */
 authority_cert_t *
-authority_cert_parse_from_string(const char *s, const char **end_of_string)
+authority_cert_parse_from_string(const char *s, size_t maxlen,
+                                 const char **end_of_string)
 {
   /** Reject any certificate at least this big; it is probably an overflow, an
    * attack, a bug, or some other nonsense. */
@@ -2319,24 +2320,25 @@ authority_cert_parse_from_string(const char *s, const char **end_of_string)
   char digest[DIGEST_LEN];
   directory_token_t *tok;
   char fp_declared[DIGEST_LEN];
-  char *eos;
+  const char *eos;
   size_t len;
   int found;
   memarea_t *area = NULL;
+  const char *end_of_s = s + maxlen;
   const char *s_dup = s;
 
-  s = eat_whitespace(s);
-  eos = strstr(s, "\ndir-key-certification");
+  s = eat_whitespace_eos(s, end_of_s);
+  eos = tor_memstr(s, end_of_s - s, "\ndir-key-certification");
   if (! eos) {
     log_warn(LD_DIR, "No signature found on key certificate");
     return NULL;
   }
-  eos = strstr(eos, "\n-----END SIGNATURE-----\n");
+  eos = tor_memstr(eos, end_of_s - eos, "\n-----END SIGNATURE-----\n");
   if (! eos) {
     log_warn(LD_DIR, "No end-of-signature found on key certificate");
     return NULL;
   }
-  eos = strchr(eos+2, '\n');
+  eos = memchr(eos+2, '\n', end_of_s - (eos+2));
   tor_assert(eos);
   ++eos;
   len = eos - s;
@@ -2353,7 +2355,7 @@ authority_cert_parse_from_string(const char *s, const char **end_of_string)
     log_warn(LD_DIR, "Error tokenizing key certificate");
     goto err;
   }
-  if (router_get_hash_impl(s, strlen(s), digest, "dir-key-certificate-version",
+  if (router_get_hash_impl(s, eos-s, digest, "dir-key-certificate-version",
                            "\ndir-key-certification", '\n', DIGEST_SHA1) < 0)
     goto err;
   tok = smartlist_get(tokens, 0);
@@ -3465,7 +3467,8 @@ networkstatus_parse_vote_from_string(const char *s,
                             "\ndir-key-certificate-version")))
       goto err;
     ++cert;
-    ns->cert = authority_cert_parse_from_string(cert, &end_of_cert);
+    ns->cert = authority_cert_parse_from_string(cert, end_of_header - cert,
+                                                &end_of_cert);
     if (!ns->cert || !end_of_cert || end_of_cert > end_of_header)
       goto err;
   }
diff --git a/src/feature/nodelist/routerparse.h b/src/feature/nodelist/routerparse.h
index 390088c948..c60046e8e8 100644
--- a/src/feature/nodelist/routerparse.h
+++ b/src/feature/nodelist/routerparse.h
@@ -93,6 +93,7 @@ smartlist_t *microdescs_parse_from_string(const char *s, const char *eos,
                                           smartlist_t *invalid_digests_out);
 
 authority_cert_t *authority_cert_parse_from_string(const char *s,
+                                                   size_t maxlen,
                                                    const char **end_of_string);
 int rend_parse_v2_service_descriptor(rend_service_descriptor_t **parsed_out,
                                      char *desc_id_out,
diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c
index 1f316ebf08..17f63e9983 100644
--- a/src/feature/relay/router.c
+++ b/src/feature/relay/router.c
@@ -717,7 +717,7 @@ load_authority_keyset(int legacy, crypto_pk_t **key_out,
                fname);
     goto done;
   }
-  parsed = authority_cert_parse_from_string(cert, &eos);
+  parsed = authority_cert_parse_from_string(cert, strlen(cert), &eos);
   if (!parsed) {
     log_warn(LD_DIR, "Unable to parse certificate in %s", fname);
     goto done;
diff --git a/src/test/test_dir.c b/src/test/test_dir.c
index 0fa5c31039..078a383dd8 100644
--- a/src/test/test_dir.c
+++ b/src/test/test_dir.c
@@ -2799,11 +2799,17 @@ test_a_networkstatus(
   MOCK(get_my_v3_authority_cert, get_my_v3_authority_cert_m);
 
   /* Parse certificates and keys. */
-  cert1 = mock_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1, NULL);
+  cert1 = mock_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1,
+                                                 strlen(AUTHORITY_CERT_1),
+                                                 NULL);
   tt_assert(cert1);
-  cert2 = authority_cert_parse_from_string(AUTHORITY_CERT_2, NULL);
+  cert2 = authority_cert_parse_from_string(AUTHORITY_CERT_2,
+                                           strlen(AUTHORITY_CERT_2),
+                                           NULL);
   tt_assert(cert2);
-  cert3 = authority_cert_parse_from_string(AUTHORITY_CERT_3, NULL);
+  cert3 = authority_cert_parse_from_string(AUTHORITY_CERT_3,
+                                           strlen(AUTHORITY_CERT_3),
+                                           NULL);
   tt_assert(cert3);
   sign_skey_1 = crypto_pk_new();
   sign_skey_2 = crypto_pk_new();
diff --git a/src/test/test_dir_common.c b/src/test/test_dir_common.c
index 6e3bb9945d..63bd082eea 100644
--- a/src/test/test_dir_common.c
+++ b/src/test/test_dir_common.c
@@ -40,14 +40,20 @@ dir_common_authority_pk_init(authority_cert_t **cert1,
 {
   /* Parse certificates and keys. */
   authority_cert_t *cert;
-  cert = authority_cert_parse_from_string(AUTHORITY_CERT_1, NULL);
+  cert = authority_cert_parse_from_string(AUTHORITY_CERT_1,
+                                          strlen(AUTHORITY_CERT_1),
+                                          NULL);
   tt_assert(cert);
   tt_assert(cert->identity_key);
   *cert1 = cert;
   tt_assert(*cert1);
-  *cert2 = authority_cert_parse_from_string(AUTHORITY_CERT_2, NULL);
+  *cert2 = authority_cert_parse_from_string(AUTHORITY_CERT_2,
+                                            strlen(AUTHORITY_CERT_2),
+                                            NULL);
   tt_assert(*cert2);
-  *cert3 = authority_cert_parse_from_string(AUTHORITY_CERT_3, NULL);
+  *cert3 = authority_cert_parse_from_string(AUTHORITY_CERT_3,
+                                            strlen(AUTHORITY_CERT_3),
+                                            NULL);
   tt_assert(*cert3);
   *sign_skey_1 = crypto_pk_new();
   *sign_skey_2 = crypto_pk_new();
diff --git a/src/test/test_dir_handle_get.c b/src/test/test_dir_handle_get.c
index 09799a0e5f..ecb7c1b5fc 100644
--- a/src/test/test_dir_handle_get.c
+++ b/src/test/test_dir_handle_get.c
@@ -1270,7 +1270,9 @@ test_dir_handle_get_server_keys_authority(void* data)
   size_t body_used = 0;
   (void) data;
 
-  mock_cert = authority_cert_parse_from_string(TEST_CERTIFICATE, NULL);
+  mock_cert = authority_cert_parse_from_string(TEST_CERTIFICATE,
+                                               strlen(TEST_CERTIFICATE),
+                                               NULL);
 
   MOCK(get_my_v3_authority_cert, get_my_v3_authority_cert_m);
   MOCK(connection_write_to_buf_impl_, connection_write_to_buf_mock);
@@ -1420,7 +1422,9 @@ test_dir_handle_get_server_keys_sk(void* data)
   size_t body_used = 0;
   (void) data;
 
-  mock_cert = authority_cert_parse_from_string(TEST_CERTIFICATE, NULL);
+  mock_cert = authority_cert_parse_from_string(TEST_CERTIFICATE,
+                                               strlen(TEST_CERTIFICATE),
+                                               NULL);
   MOCK(get_my_v3_authority_cert, get_my_v3_authority_cert_m);
   MOCK(connection_write_to_buf_impl_, connection_write_to_buf_mock);
 
@@ -2388,7 +2392,9 @@ test_dir_handle_get_status_vote_next_authority(void* data)
   routerlist_free_all();
   dirvote_free_all();
 
-  mock_cert = authority_cert_parse_from_string(TEST_CERTIFICATE, NULL);
+  mock_cert = authority_cert_parse_from_string(TEST_CERTIFICATE,
+                                               strlen(TEST_CERTIFICATE),
+                                               NULL);
 
   /* create a trusted ds */
   ds = trusted_dir_server_new("ds", "127.0.0.1", 9059, 9060, NULL, digest,
@@ -2466,7 +2472,9 @@ test_dir_handle_get_status_vote_current_authority(void* data)
   routerlist_free_all();
   dirvote_free_all();
 
-  mock_cert = authority_cert_parse_from_string(TEST_CERTIFICATE, NULL);
+  mock_cert = authority_cert_parse_from_string(TEST_CERTIFICATE,
+                                               strlen(TEST_CERTIFICATE),
+                                               NULL);
 
   /* create a trusted ds */
   ds = trusted_dir_server_new("ds", "127.0.0.1", 9059, 9060, NULL, digest,
diff --git a/src/test/test_routerlist.c b/src/test/test_routerlist.c
index bf76570b3f..e7c577cf66 100644
--- a/src/test/test_routerlist.c
+++ b/src/test/test_routerlist.c
@@ -260,7 +260,9 @@ test_router_pick_directory_server_impl(void *arg)
 
   /* Init SR subsystem. */
   MOCK(get_my_v3_authority_cert, get_my_v3_authority_cert_m);
-  mock_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1, NULL);
+  mock_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1,
+                                               strlen(AUTHORITY_CERT_1),
+                                               NULL);
   sr_init(0);
   UNMOCK(get_my_v3_authority_cert);
 
@@ -472,7 +474,9 @@ test_directory_guard_fetch_with_no_dirinfo(void *arg)
 
   /* Initialize the SRV subsystem */
   MOCK(get_my_v3_authority_cert, get_my_v3_authority_cert_m);
-  mock_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1, NULL);
+  mock_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1,
+                                               strlen(AUTHORITY_CERT_1),
+                                               NULL);
   sr_init(0);
   UNMOCK(get_my_v3_authority_cert);
 
@@ -645,7 +649,9 @@ test_skew_common(void *arg, time_t now, unsigned long *offset)
 
   /* Initialize the SRV subsystem */
   MOCK(get_my_v3_authority_cert, get_my_v3_authority_cert_m);
-  mock_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1, NULL);
+  mock_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1,
+                                               strlen(AUTHORITY_CERT_1),
+                                               NULL);
   sr_init(0);
   UNMOCK(get_my_v3_authority_cert);
 
diff --git a/src/test/test_shared_random.c b/src/test/test_shared_random.c
index 70adf580ab..2043613641 100644
--- a/src/test/test_shared_random.c
+++ b/src/test/test_shared_random.c
@@ -64,7 +64,9 @@ init_authority_state(void)
   MOCK(get_my_v3_authority_cert, get_my_v3_authority_cert_m);
 
   or_options_t *options = get_options_mutable();
-  mock_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1, NULL);
+  mock_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1,
+                                               strlen(AUTHORITY_CERT_1),
+                                               NULL);
   tt_assert(mock_cert);
   options->AuthoritativeDir = 1;
   tt_int_op(load_ed_keys(options, time(NULL)), OP_GE, 0);
@@ -420,7 +422,9 @@ test_sr_commit(void *arg)
   {  /* Setup a minimal dirauth environment for this test  */
     or_options_t *options = get_options_mutable();
 
-    auth_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1, NULL);
+    auth_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1,
+                                                 strlen(AUTHORITY_CERT_1),
+                                                 NULL);
     tt_assert(auth_cert);
 
     options->AuthoritativeDir = 1;
@@ -823,7 +827,9 @@ test_sr_setup_commits(void)
   {  /* Setup a minimal dirauth environment for this test  */
     or_options_t *options = get_options_mutable();
 
-    auth_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1, NULL);
+    auth_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1,
+                                                 strlen(AUTHORITY_CERT_1),
+                                                 NULL);
     tt_assert(auth_cert);
 
     options->AuthoritativeDir = 1;