Multiple fixes for the HSFETCH command

Ref:
https://trac.torproject.org/projects/tor/ticket/14847?replyto=31#comment:31

Signed-off-by: David Goulet <dgoulet@ev0ke.net>

1 files changed, 26 insertions, 24 deletions

diff --git a/src/or/control.c b/src/or/control.c
index 10475bb271..b4d671bea8 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -3263,28 +3263,27 @@ static int
 handle_control_hsfetch(control_connection_t *conn, uint32_t len,
                        const char *body)
 {
+  int i;
   char digest[DIGEST_LEN], *hsaddress = NULL, *arg1 = NULL, *desc_id = NULL;
   smartlist_t *args = NULL, *hsdirs = NULL;
   (void) len; /* body is nul-terminated; it's safe to ignore the length */
+  static const char *hsfetch_command = "HSFETCH";
   static const char *v2_str = "v2-";
   const size_t v2_str_len = strlen(v2_str);
   rend_data_t *rend_query = NULL;
 
   /* Make sure we have at least one argument, the HSAddress. */
-  args = getargs_helper("HSFETCH", conn, body, 1, -1);
+  args = getargs_helper(hsfetch_command, conn, body, 1, -1);
   if (!args) {
-    goto done;
+    goto exit;
   }
 
   /* Extract the first argument (either HSAddress or DescID). */
   arg1 = smartlist_get(args, 0);
-  /* Remove it from the argument list so we can safely iterate on all the
-   * rest to find optional argument(s). */
-  smartlist_del(args, 0);
   /* Test if it's an HS address without the .onion part. */
   if (rend_valid_service_id(arg1)) {
     hsaddress = arg1;
-  } else if (strstr(arg1, v2_str) &&
+  } else if (strcmpstart(arg1, v2_str) == 0 &&
              strlen(arg1 + v2_str_len) == REND_DESC_ID_V2_LEN_BASE32 &&
              base32_decode(digest, sizeof(digest), arg1 + v2_str_len,
                            REND_DESC_ID_V2_LEN_BASE32) == 0) {
@@ -3292,15 +3291,17 @@ handle_control_hsfetch(control_connection_t *conn, uint32_t len,
      * of the id. */
     desc_id = digest;
   } else {
-    connection_printf_to_buf(conn, "552 Unrecognized \"%s\"\r\n",
+    connection_printf_to_buf(conn, "513 Unrecognized \"%s\"\r\n",
                              arg1);
     goto done;
   }
 
-  /* Skip first argument because it's the HSAddress. */
-  SMARTLIST_FOREACH_BEGIN(args, char *, arg) {
+  static const char *opt_server = "SERVER=";
+
+  /* Skip first argument because it's the HSAddress or DescID. */
+  for (i = 1; i < smartlist_len(args); ++i) {
+    const char *arg = smartlist_get(args, i);
     const node_t *node;
-    static const char *opt_server = "SERVER=";
 
     if (!strcasecmpstart(arg, opt_server)) {
       const char *server;
@@ -3319,11 +3320,11 @@ handle_control_hsfetch(control_connection_t *conn, uint32_t len,
       /* Valid server, add it to our local list. */
       smartlist_add(hsdirs, node->rs);
     } else {
-      connection_printf_to_buf(conn, "552 Unexpected argument \"%s\"\r\n",
+      connection_printf_to_buf(conn, "513 Unexpected argument \"%s\"\r\n",
                                arg);
       goto done;
     }
-  } SMARTLIST_FOREACH_END(arg);
+  }
 
   rend_query = tor_malloc_zero(sizeof(*rend_query));
 
@@ -3334,7 +3335,8 @@ handle_control_hsfetch(control_connection_t *conn, uint32_t len,
     /* Using a descriptor ID, we force the user to provide at least one
      * hsdir server using the SERVER= option. */
     if (!hsdirs || !smartlist_len(hsdirs)) {
-      connection_printf_to_buf(conn, "552 SERVER= option is required\r\n");
+      connection_printf_to_buf(conn, "512 %s option is required\r\n",
+                               opt_server);
       goto done;
     }
     memcpy(rend_query->descriptor_id, desc_id,
@@ -3354,15 +3356,12 @@ handle_control_hsfetch(control_connection_t *conn, uint32_t len,
   rend_client_fetch_v2_desc(rend_query, hsdirs);
 
 done:
-  if (hsdirs) {
-    smartlist_free(hsdirs);
-  }
-  if (args) {
-    SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
-    smartlist_free(args);
-  }
+  SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
+  smartlist_free(args);
+  /* Contains data pointer that we don't own thus no cleanup. */
+  smartlist_free(hsdirs);
   tor_free(rend_query);
-  tor_free(arg1);
+exit:
   return 0;
 }
 
@@ -5470,13 +5469,16 @@ control_event_hs_descriptor_content(const char *onion_address,
   static const char *event_name = "HS_DESC_CONTENT";
   char *esc_content = NULL;
 
-  if (!onion_address || !desc_id || !hsdir_id_digest || !content) {
+  if (!onion_address || !desc_id || !hsdir_id_digest) {
     log_warn(LD_BUG, "Called with onion_address==%p, desc_id==%p, "
-             "hsdir_id_digest==%p, content==%p", onion_address, desc_id,
-             hsdir_id_digest, content);
+             "hsdir_id_digest==%p", onion_address, desc_id, hsdir_id_digest);
     return;
   }
 
+  if (content == NULL) {
+    /* Point it to empty content so it can still be escaped. */
+    content = "";
+  }
   write_escaped_data(content, strlen(content), &esc_content);
 
   send_control_event(EVENT_HS_DESC_CONTENT, ALL_FORMATS,