GitLab

In the main loop of networkstatus_compute_consensus, we do not reset rs_out between iterations. That wasn't a problem before consensus method 5, where we set every field in rs_out during every iteration of the loop. But this block here is a problem:

      if (consensus_method >= 6 && num_mbws > 2) {
        rs_out.has_bandwidth = 1;
        rs_out.bandwidth = median_uint32(measured_bws, num_mbws);
      } else if (consensus_method >= 5 && num_bandwidths > 0) {
        rs_out.has_bandwidth = 1;
        rs_out.bandwidth = median_uint32(bandwidths, num_bandwidths);
      }

Fortunately, we set rs_out.has_bandwidth = 0 earlier in the loop, so it is only rs_out.bandwidth that leaks from iteration to iteration. As far as I can tell, we only look at rs_out.bandwidth when has_bandwidth is set. So that's okay.

Also see:

        if (chosen_exitsummary) {
          rs_out.has_exitsummary = 1;
          /* yea, discards the const */
          rs_out.exitsummary = (char *)chosen_exitsummary;
        }

Again, has_exitsummary is reset earlier in the loop, but exitsummary isn't inspected unless has_exitsummary is set.

Nevertheless, this logic is an accident waiting to happen.

Reported pseudonymously; I am not 100% convinced there is not a bug here that I am missing.