commit f632335feb27b45a3ee5eb64690826bda52467bd Author: Nick Mathewson nickm@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;