GitLab

There are various instances where we combine sr_commit_get_rsa_fpr() with hex_str() in log messages. This won't work because sr_commit_get_rsa_fpr() actually calls hex_str() underneath, and you can't have repeated calls to hex_str() because it uses a stack buffer.

Examples:

  log_debug(LD_DIR, "SR: Inspecting commit from %s (voter: %s)?",
            sr_commit_get_rsa_fpr(commit),
            hex_str(voter_key, DIGEST_LEN));

    if (!commitments_are_the_same(commit, saved_commit)) {
      log_warn(LD_DIR, "SR: Commit from authority %s is different from "
                       "previous commit in our state (voter: %s)",
               sr_commit_get_rsa_fpr(commit),
               hex_str(voter_key, DIGEST_LEN));
      goto ignore;
    }

{{{
if (verify_commit_and_reveal(commit) < 0) { log_warn(LD_BUG, "SR:a Commit from authority %s has an invalid " "reveal value. (voter: %s)", sr_commit_get_rsa_fpr(commit), hex_str(voter_key, DIGEST_LEN)); goto ignore; } }}}

I found this issue while auditing log messages that did not make sense in a Chutney network.

In general, it's naughty to hide a delicate function like hex_str() behind other functions (like sr_commit_get_rsa_fpr()) without making it clear that this is the case.

Fortunately, from what I see, this bug only occurs in logging cases. But we should make sure we don't have this sort of issue from now on.