Potential heap corruption in do_getpass in routerkeys.c

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.