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).
This commit is contained in:
George Kadianakis 2017-08-28 16:30:51 +03:00
parent d4f1b566e1
commit 93a0a4a422

View File

@ -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;
}