[tor-bugs] #14013 [Tor]: base16_decode() API is inconsistent and error-prone

[Tor Bug Tracker & Wiki]

#14013: base16_decode() API is inconsistent and error-prone
-------------------------------------------------+-------------------------
 Reporter:  nickm                                |          Owner:
     Type:  defect                               |         Status:  new
 Priority:  major                                |      Milestone:  Tor:
Component:  Tor                                  |  0.2.6.x-final
 Keywords:  024-backport 025-backport tor-       |        Version:
  client                                         |  Actual Points:
Parent ID:                                       |         Points:
-------------------------------------------------+-------------------------
 Our other baseXYZ_decode() functions return the number of bytes written to
 the output.  But base16_decode() returns 0 on success and -1 on failure,
 so unless you checked the input length, you have a risk of leaving the
 output buffer partially full.

 Calls where we messed up:
    * connction_dir_retry_bridges()
    * decoding the K_FINGERPRINT entry in router_parse_entry_from_string()
    * add_fingerprint_to_dir()
    * entry_Guards_parse_state()
    * getinfo_helper_networkstatus()
    * authority_cert_parse_from_string()

 As far as I can tell, none of these is disastrous, or leaks stack
 information to an attacker.  But wow, it sure would be good to fix them.

 Proposed fix for backporting:
    * Add a memset(0) to base*_en/decode, so they never leave stuff
 unassigned.

 Proposed fix for master:
    * Make the return value for base16_decode consistent with the others.
    * Fix everything that checks for whether base16_decode() to check an
 actual return value.

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