[tor-bugs] #17682 [Tor]: safe_timer_diff is unsafe under wrapping

[Tor Bug Tracker & Wiki]

#17682: safe_timer_diff is unsafe under wrapping
------------------------------+------------------------------------
 Reporter:  teor              |          Owner:  nickm
     Type:  defect            |         Status:  needs_revision
 Priority:  High              |      Milestone:  Tor: 0.2.8.x-final
Component:  Tor               |        Version:  Tor: unspecified
 Severity:  Normal            |     Resolution:
 Keywords:  regression, easy  |  Actual Points:
Parent ID:  #17983            |         Points:  small
  Sponsor:                    |
------------------------------+------------------------------------
Changes (by teor):

 * status:  needs_review => needs_revision


Comment:

 Code review:

 {{{
 +    /* There were no computers at signed TIME_MIN, and nothing that could
 run
 +     * Tor. It's a bug if 'now' is around then. */
 +    tor_assert(now > TIME_MIN + LONGEST_TIMER_PERIOD);
 }}}
 Doesn't shadow rely on starting Tor at the epoch?
 (I think I heard that once, I'm not sure how accurate it is.)

 {{{
 +    if (next < now - LONGEST_TIMER_PERIOD)
 +      return LONGEST_TIMER_PERIOD;
 }}}
 Do you mean `next > now - LONGEST_TIMER_PERIOD`?

 Given the assertion, `now - LONGEST_TIMER_PERIOD` never underflows, so
 `now - LONGEST_TIMER_PERIOD > TIME_MIN`.
 And if `next > now`, then it's never true that `next < now -
 LONGEST_TIMER_PERIOD`.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/17682#comment:12>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online