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).

diff --git a/src/or/hs_common.c b/src/or/hs_common.c
index 03dd07f6cac3cb199f73158d3fe243273174a076..a4b8ba3e8d3ba893e7d83cab63814bbb9e0f5162 100644 (file)
--- 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;
     }