commit 93a0a4a4221b5c60b3c4fe22d38c81914176dedb Author: George Kadianakis <desnacked@riseup.net> Date: Mon Aug 28 16:30:51 2017 +0300 prop224: Fix length check when purging hidserv requests. That check was wrong: a) We should be making sure that the size of `key` is big enough before proceeding, since that's the buffer that we would overread with the tor_memeq() below. The old check used to check that `req_key_str` is big enough which is not right, since we won't read deep into that buffer. The new check makes sure that `key` has enough size to survive the tor_memeq(), and if not it moves to the next element of the strmap. b) That check shouldn't be a BUG since that strmap contains variable-sized elements and we should not be bugging out if we happen to compare a small sized element (v2) to a bigger one (v3). --- src/or/hs_common.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/or/hs_common.c b/src/or/hs_common.c index 03dd07f6c..a4b8ba3e8 100644 --- a/src/or/hs_common.c +++ b/src/or/hs_common.c @@ -1453,14 +1453,12 @@ hs_purge_hid_serv_from_last_hid_serv_requests(const char *req_key_str) /* XXX: The use of REND_DESC_ID_V2_LEN_BASE32 is very wrong in terms of * semantic, see #23305. */ - /* Length check on the strings we are about to compare. The "key" contains - * both the base32 HSDir identity digest and the requested key at the + /* This strmap contains variable-sized elements so this is a basic length + * check on the strings we are about to compare. The "key" contains both + * the base32 HSDir identity digest and the requested key at the * directory. The "req_key_str" can either be a base32 descriptor ID or a - * base64 blinded key which should be the second part of "key". BUG on - * this check because both strings are internally controlled so this - * should never happen. */ - if (BUG((strlen(req_key_str) + REND_DESC_ID_V2_LEN_BASE32) < - strlen(key))) { + * base64 blinded key which should be the second part of "key". */ + if (strlen(key) < REND_DESC_ID_V2_LEN_BASE32 + strlen(req_key_str)) { iter = strmap_iter_next(last_hid_serv_requests, iter); continue; }