base16_decode() API is inconsistent and error-prone

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.