[tor-commits] [tor/master] Fix users of base32_decode to check for expected length in return.

asn at torproject.org asn at torproject.org
Tue Feb 26 10:34:16 UTC 2019


commit f632335feb27b45a3ee5eb64690826bda52467bd
Author: Nick Mathewson <nickm at torproject.org>
Date:   Thu Jan 17 13:32:19 2019 -0500

    Fix users of base32_decode to check for expected length in return.
    
    Also, when we log about a failure from base32_decode(), we now
    say that the length is wrong or that the characters were invalid:
    previously we would just say that there were invalid characters.
    
    Follow-up on 28913 work.
---
 src/feature/hs/hs_client.c    |  5 ++++-
 src/feature/hs/hs_common.c    |  3 ++-
 src/feature/hs/hs_service.c   |  3 ++-
 src/feature/rend/rendcache.c  |  7 ++++---
 src/feature/rend/rendcommon.c |  5 +++--
 src/feature/rend/rendparse.c  | 17 +++++++++++------
 6 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c
index dfad216ab..a8a4aa776 100644
--- a/src/feature/hs/hs_client.c
+++ b/src/feature/hs/hs_client.c
@@ -1517,7 +1517,10 @@ parse_auth_file_content(const char *client_key_str)
   auth = tor_malloc_zero(sizeof(hs_client_service_authorization_t));
   if (base32_decode((char *) auth->enc_seckey.secret_key,
                     sizeof(auth->enc_seckey.secret_key),
-                    seckey_b32, strlen(seckey_b32)) < 0) {
+                    seckey_b32, strlen(seckey_b32)) !=
+      sizeof(auth->enc_seckey.secret_key)) {
+    log_warn(LD_REND, "Client authorization encoded base32 private key "
+                      "can't be decoded: %s", seckey_b32);
     goto err;
   }
   strncpy(auth->onion_address, onion_address, HS_SERVICE_ADDR_LEN_BASE32);
diff --git a/src/feature/hs/hs_common.c b/src/feature/hs/hs_common.c
index 8dbd9485e..97229ac11 100644
--- a/src/feature/hs/hs_common.c
+++ b/src/feature/hs/hs_common.c
@@ -926,7 +926,8 @@ hs_parse_address(const char *address, ed25519_public_key_t *key_out,
   }
 
   /* Decode address so we can extract needed fields. */
-  if (base32_decode(decoded, sizeof(decoded), address, strlen(address)) < 0) {
+  if (base32_decode(decoded, sizeof(decoded), address, strlen(address))
+      != sizeof(decoded)) {
     log_warn(LD_REND, "Service address %s can't be decoded.",
              escaped_safe_str(address));
     goto invalid;
diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c
index 623a239d5..6f6cf0105 100644
--- a/src/feature/hs/hs_service.c
+++ b/src/feature/hs/hs_service.c
@@ -1179,7 +1179,8 @@ parse_authorized_client(const char *client_key_str)
   client = tor_malloc_zero(sizeof(hs_service_authorized_client_t));
   if (base32_decode((char *) client->client_pk.public_key,
                     sizeof(client->client_pk.public_key),
-                    pubkey_b32, strlen(pubkey_b32)) < 0) {
+                    pubkey_b32, strlen(pubkey_b32)) !=
+      sizeof(client->client_pk.public_key)) {
     log_warn(LD_REND, "Client authorization public key cannot be decoded: %s",
              pubkey_b32);
     goto err;
diff --git a/src/feature/rend/rendcache.c b/src/feature/rend/rendcache.c
index ecd85e4a5..699b5a4a4 100644
--- a/src/feature/rend/rendcache.c
+++ b/src/feature/rend/rendcache.c
@@ -593,10 +593,10 @@ rend_cache_lookup_v2_desc_as_dir(const char *desc_id, const char **desc)
   char desc_id_digest[DIGEST_LEN];
   tor_assert(rend_cache_v2_dir);
   if (base32_decode(desc_id_digest, DIGEST_LEN,
-                    desc_id, REND_DESC_ID_V2_LEN_BASE32) < 0) {
+                    desc_id, REND_DESC_ID_V2_LEN_BASE32) != DIGEST_LEN) {
     log_fn(LOG_PROTOCOL_WARN, LD_REND,
            "Rejecting v2 rendezvous descriptor request -- descriptor ID "
-           "contains illegal characters: %s",
+           "has wrong length or illegal characters: %s",
            safe_str(desc_id));
     return -1;
   }
@@ -854,7 +854,8 @@ rend_cache_store_v2_desc_as_client(const char *desc,
     *entry = NULL;
   }
   if (base32_decode(want_desc_id, sizeof(want_desc_id),
-                    desc_id_base32, strlen(desc_id_base32)) < 0) {
+                    desc_id_base32, strlen(desc_id_base32)) !=
+      sizeof(want_desc_id)) {
     log_warn(LD_BUG, "Couldn't decode base32 %s for descriptor id.",
              escaped_safe_str_client(desc_id_base32));
     goto err;
diff --git a/src/feature/rend/rendcommon.c b/src/feature/rend/rendcommon.c
index 15e4534fc..80edf549f 100644
--- a/src/feature/rend/rendcommon.c
+++ b/src/feature/rend/rendcommon.c
@@ -171,9 +171,10 @@ rend_compute_v2_desc_id(char *desc_id_out, const char *service_id,
   }
   /* Convert service ID to binary. */
   if (base32_decode(service_id_binary, REND_SERVICE_ID_LEN,
-                    service_id, REND_SERVICE_ID_LEN_BASE32) < 0) {
+                    service_id, REND_SERVICE_ID_LEN_BASE32) !=
+      REND_SERVICE_ID_LEN) {
     log_warn(LD_REND, "Could not compute v2 descriptor ID: "
-                      "Illegal characters in service ID: %s",
+                      "Illegal characters or wrong length for service ID: %s",
              safe_str_client(service_id));
     return -1;
   }
diff --git a/src/feature/rend/rendparse.c b/src/feature/rend/rendparse.c
index e2378e340..c79f861b5 100644
--- a/src/feature/rend/rendparse.c
+++ b/src/feature/rend/rendparse.c
@@ -143,8 +143,9 @@ rend_parse_v2_service_descriptor(rend_service_descriptor_t **parsed_out,
     goto err;
   }
   if (base32_decode(desc_id_out, DIGEST_LEN,
-                    tok->args[0], REND_DESC_ID_V2_LEN_BASE32) < 0) {
-    log_warn(LD_REND, "Descriptor ID contains illegal characters: %s",
+                    tok->args[0], REND_DESC_ID_V2_LEN_BASE32) != DIGEST_LEN) {
+    log_warn(LD_REND,
+             "Descriptor ID has wrong length or illegal characters: %s",
              tok->args[0]);
     goto err;
   }
@@ -174,8 +175,10 @@ rend_parse_v2_service_descriptor(rend_service_descriptor_t **parsed_out,
     log_warn(LD_REND, "Invalid secret ID part: '%s'", tok->args[0]);
     goto err;
   }
-  if (base32_decode(secret_id_part, DIGEST_LEN, tok->args[0], 32) < 0) {
-    log_warn(LD_REND, "Secret ID part contains illegal characters: %s",
+  if (base32_decode(secret_id_part, DIGEST_LEN, tok->args[0], 32) !=
+      DIGEST_LEN) {
+    log_warn(LD_REND,
+             "Secret ID part has wrong length or illegal characters: %s",
              tok->args[0]);
     goto err;
   }
@@ -429,8 +432,10 @@ rend_parse_introduction_points(rend_service_descriptor_t *parsed,
     /* Parse identifier. */
     tok = find_by_keyword(tokens, R_IPO_IDENTIFIER);
     if (base32_decode(info->identity_digest, DIGEST_LEN,
-                      tok->args[0], REND_INTRO_POINT_ID_LEN_BASE32) < 0) {
-      log_warn(LD_REND, "Identity digest contains illegal characters: %s",
+                      tok->args[0], REND_INTRO_POINT_ID_LEN_BASE32) !=
+        DIGEST_LEN) {
+      log_warn(LD_REND,
+               "Identity digest has wrong length or illegal characters: %s",
                tok->args[0]);
       rend_intro_point_free(intro);
       goto err;





More information about the tor-commits mailing list