[tor-bugs] #3683 [Tor Client]: Stream-isolation code does not handle NULs in SOCKS auth fields properly

[Tor Bug Tracker & Wiki]

#3683: Stream-isolation code does not handle NULs in SOCKS auth fields properly
------------------------+---------------------------------------------------
 Reporter:  rransom     |          Owner:  nickm             
     Type:  defect      |         Status:  needs_review      
 Priority:  normal      |      Milestone:  Tor: 0.2.3.x-final
Component:  Tor Client  |        Version:                    
 Keywords:              |         Parent:                    
   Points:              |   Actualpoints:                    
------------------------+---------------------------------------------------

Comment(by rransom):

 Replying to [comment:1 nickm]:
 > Yikes.  Glad that never saw production.
 >
 > Possible fix in branch bug3683 in my public repository.
 >
 > Should-I-care questions: The memcmp is only data-independent under
 limited circumstances: if either input is NULL, or if their lengths vary,
 it returns faster than if they are both strings of the same length.

 There isn't much we can do about that except hash/HMAC the username and
 password immediately and only store and compare hashes, and that sounds
 like more trouble than it's currently worth.

 Since we're treating the SOCKS authentication values as potentially
 sensitive, we should also (try to) zero them in `socks_request_free`.

 > Also, I think that the use of uint8_t for usernamelen/socks_username_len
 might be wrong; socks4 authenticators are NUL-terminated IIRC, not length-
 extent?

 Yes.  Fortunately, the integer overflow that produced in `parse_socks`
 seems to be relatively harmless.

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