GitLab

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.