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.
issue