Re: [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:  closed
 Priority:  Low  |  Milestone:  Tor:
 |  0.2.9.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  unspecified
 Severity:  Normal   | Resolution:  fixed
 Keywords:  tor-bug-bounty, 028-backport,|  Actual Points:
  isaremoved, nickwants029, review-group-10  |
Parent ID:   | Points:  0.5
 Reviewer:   |Sponsor:
-+-
Changes (by nickm):

 * status:  merge_ready => closed
 * resolution:   => fixed


Comment:

 lgtm; merged!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:
 |  merge_ready
 Priority:  Low  |  Milestone:  Tor:
 |  0.2.9.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  unspecified
 Severity:  Normal   | Resolution:
 Keywords:  tor-bug-bounty, 028-backport,|  Actual Points:
  isaremoved, nickwants029, review-group-10  |
Parent ID:   | Points:  0.5
 Reviewer:   |Sponsor:
-+-
Changes (by asn):

 * status:  needs_review => merge_ready


Comment:

 Hello,

 I reviewed nherring's patch and it seems alright. I also tested it against
 Guido's PoC and ASAN does not crash anymore.

 BTW, since no branch was provided, I pushed nherring's patch on my repo as
 `bug19223` and also added a changes file. Please check it out.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:
 |  needs_review
 Priority:  Low  |  Milestone:  Tor:
 |  0.2.9.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  unspecified
 Severity:  Normal   | Resolution:
 Keywords:  tor-bug-bounty, 028-backport,|  Actual Points:
  isaremoved nickwants029|
Parent ID:   | Points:  0.5
 Reviewer:   |Sponsor:
-+-
Changes (by nickm):

 * status:  new => needs_review
 * milestone:  Tor: 0.2.??? => Tor: 0.2.9.x-final


Comment:

 Hi! Ive marked this for review, and since it's small, I've marked it for
 potential inclusion in 0.2.9.

 For more information about tests, code review, submission, etc, look at
 the doc/HACKING subdirectory of the Tor source tree, especially
 doc/HACKING/GettingStarted.md

 Thanks!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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   | Resolution:
 Keywords:  tor-bug-bounty, 028-backport,|  Actual Points:
  isaremoved nickwants029|
Parent ID:   | Points:  0.5
 Reviewer:   |Sponsor:
-+-

Comment (by nherring):

 Have a suggested fix, but don't know the model for adding tests, code
 review, submission, etc. Ptr to FAQ/instructions appreciated.

 {{{
 $ git diff src/or/routerkeys.c
 diff --git a/src/or/routerkeys.c b/src/or/routerkeys.c
 index 060ffd8..d5e7051 100644
 --- a/src/or/routerkeys.c
 +++ b/src/or/routerkeys.c
 @@ -48,8 +48,8 @@ do_getpass(const char *prompt, char *buf, size_t buflen,
  size_t p2len = strlen(prompt) + 1;
  if (p2len < sizeof(msg))
p2len = sizeof(msg);
 -prompt2 = tor_malloc(strlen(prompt)+1);
 -memset(prompt2, ' ', p2len);
 +prompt2 = tor_malloc(p2len);
 +memset(prompt2, ' ', p2len - sizeof(msg));
  memcpy(prompt2 + p2len - sizeof(msg), msg, sizeof(msg));

  buf2 = tor_malloc_zero(buflen);
 }}}

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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   | Resolution:
 Keywords:  tor-bug-bounty, 028-backport,|  Actual Points:
  isaremoved nickwants029|
Parent ID:   | Points:  0.5
 Reviewer:   |Sponsor:
-+-
Changes (by nickm):

 * keywords:  tor-bug-bounty, 028-backport, isaremoved => tor-bug-bounty,
 028-backport, isaremoved nickwants029


Comment:

 This would IMO be even less than .5 points to fix, and it implies a
 potential security issue down the road if we don't.  (We would be kicking
 ourselves if this ever bit us.)

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:
Component:  Core Tor/Tor |  0.2.9.x-final
 Severity:  Normal   |Version:  Tor:
 Keywords:  tor-bug-bounty 028-backport  |  unspecified
Parent ID:   | Resolution:
 Reviewer:   |  Actual Points:
 | Points:  0.5
 |Sponsor:
-+-
Changes (by nickm):

 * keywords:  tor-bug-bounty => tor-bug-bounty 028-backport
 * milestone:  Tor: 0.2.??? => Tor: 0.2.9.x-final


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

[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
 0x6004dffe 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: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs