[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:                    |
------------------------------+------------------------------------

Comment (by nickm):

 Replying to [comment:12 teor]:
 > 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.)

 I believe it once did.  But in any case, TIME_MIN on a 32-bit computer
 isn't 1970 -- it's 1902.  And on a 64-bit time_t, it is 292 billion years
 in the past, whereas current cosmological models only put our universe at
 13.8 billion years old.

 (I continue to maintained that unsigned time_t doesn't actually exist.
 Adding a ticket to remove unsigned TIME_T support as #18184 )

 > {{{
 > +    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`.

 D'oh.  Trying again.  I think the fixup in `bug17682` should be right.

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