Re: [tor-bugs] #12971 [Tor]: Invalid SOCKS5 response to UDP associate request

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#12971: Invalid SOCKS5 response to UDP associate request
-----------------------------+------------------------------
     Reporter:  yurivict271  |      Owner:  rl1987
         Type:  defect       |     Status:  needs_revision
     Priority:  normal       |  Milestone:  Tor: 0.2.???
    Component:  Tor          |    Version:
   Resolution:               |   Keywords:  tor-client, easy
Actual Points:               |  Parent ID:
       Points:               |
-----------------------------+------------------------------
Changes (by yawning):

 * status:  needs_review => needs_revision


Comment:

 A few thoughts:
  * A better fix would be to check if CMD is any of the commands we
 actually support, instead of explicitly checking if it is UDP ASSOCIATE.
 I could edit the test case to issue a BIND (also unsupported), and it
 wouldn't send the correct response.  Note that this doesn't mean, also
 check BIND, what would happen if I send a request with CMD=0x23?
  * The blurb to generate the response should be a function.  `static int
 send_socks5_error(socks_request_t *req, socks5_reply_status_t reason);`
 would be what I would use.
  * `0x07` -> SOCKS5_COMMAND_NOT_SUPPORTED (If you use the prototype I
 suggested, this should be obvious).
  * Adding a check for UDP_ASSOCIATE to the if is incorrect, execution will
 never reach there, since you're examining the command earlier in the
 function, and returning.
  * Line 1996 (your copy), gratuitous whitespace change.

 It would be nice after carving out the error generation if you looked at
 other places where we reject requests and sent back sensible errors.  For
 example:
 {{{
           log_warn(LD_APP,"socks5: unsupported address type %d.
 Rejecting.",
                    (int) *(data+3));
           return send_socks5_error(req,
 SOCKS5_ADDRESS_TYPE_NOT_SUPPORTED);
           /* NB: I assume send_socks5_error always returns -1, could also
 make it a void and have a separate return, but this is more concise. */
 }}}

 For cases where none of the more specific error reasons are applicable,
 `SOCKS5_GENERAL_ERROR` should be used.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/12971#comment:12>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs