[tor-bugs] #19223 [Core Tor/Tor]: Potential heap corruption in do_getpass in routerkeys.c

[Tor Bug Tracker & Wiki]

#19223: Potential heap corruption in do_getpass in routerkeys.c
------------------------------+------------------------------
     Reporter:  asn           |      Owner:
         Type:  defect        |     Status:  new
     Priority:  Low           |  Milestone:  Tor: 0.2.???
    Component:  Core Tor/Tor  |    Version:  Tor: unspecified
     Severity:  Normal        |   Keywords:  tor-bug-bounty
Actual Points:                |  Parent ID:
       Points:  0.5           |   Reviewer:
      Sponsor:                |
------------------------------+------------------------------
 Hello,

 this is a bug by Guido Vranken from our bug bounty program. This bug is
 not triggerable in the current codebase, but it's still a good idea to
 fix, for future safety.

 Here follows the bug report as received:

 ----

 `do_getpass` contains this code:
 {{{
   if (twice) {
     const char msg[] = "One more time:";
     size_t p2len = strlen(prompt) + 1;
     if (p2len < sizeof(msg))
       p2len = sizeof(msg);
     prompt2 = tor_malloc(strlen(prompt)+1);
     memset(prompt2, ' ', p2len);
     memcpy(prompt2 + p2len - sizeof(msg), msg, sizeof(msg));

     buf2 = tor_malloc_zero(buflen);
   }
 }}}

 There is only one call to this function in the code for which twice == 1:

 {{{
   if (do_getpass("Enter new passphrase:", pwbuf0, sizeof(pwbuf0), 1,
                  get_options()) < 0) {
     log_warn(LD_OR, "NO/failed passphrase");
     return -1;
   }
 }}}

 This will not trigger a memory corruption, but if the first parameter had
 been shorter, it would:

 Compile and run like this:

 {{{
 $ gcc -fomit-frame-pointer -fsanitize=address do_getpass.c
 $ ./a.out "Enter new passphrase:"
 $ ./a.out "Enter new passphrase"
 $ ./a.out "Enter new passphras"
 $ ./a.out "Enter new passphra"
 $ ./a.out "Enter new passphr"
 $ ./a.out "Enter new passph"
 $ ./a.out "Enter new passp"
 $ ./a.out "Enter new pass"
 $ ./a.out "Enter new pas"

 ==7883== ERROR: AddressSanitizer: heap-buffer-overflow on address
 0x60040000dffe at pc 0x400c0a bp 0x7fff8d9c22e0 sp 0x7fff8d9c22d8
 ...
 ...
 }}}

 So it's not really a vulnerability at present, but I thought I'd mention
 it to
 you since it struck me as odd and it could become a problem if you pass a
 dynamic, potentially short string (for ex. created with snprintf) to
 do_getpass.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/19223>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online