#3880: integer overflow in date.c (mutt_mktime)
-----------------------+----------------------
Reporter: vinc17 | Owner: mutt-dev
Type: defect | Status: new
Priority: critical | Milestone:
Component: mutt | Version: 1.7.0
Resolution: | Keywords:
-----------------------+----------------------
Comment (by vinc17):
Replying to [comment:10 code@…]:
> Well, so I did some research on this, and it turns out that portably
> and reliably calculating the maximum value of an integer type is
> actually impossible. In addition to CHAR_BIT, there's also the matter
> of padding bits,
Yes, this is what I've said in comment:7.
> and the fact that which bit is the sign bit is implementation-specific.
It is always the most significant bit of the value bits. But this
information is useless if one doesn't know the number of padding bits.
Anyway...
> In that light, Vincent's macro is probably sufficient...
Yes, as all practical implementations do not have padding bits (and it
seems that the concept of padding bits is more or less obsolete, so that
padding bits are not likely to come back).
> However I actually think the right solution to this problem is to do
> nothing. So what if the time_t overflows? Why complicate the code to
> jump through hoops for this? Displaying an e-mail that has an invalid
> date with an incorrect date seems like about all you could do
> anyway
The current behavior with security compiler options such as UBsan is that
Mutt aborts, which is not acceptable.
Moreover, even without such options, there could be issues in the future.
First, there may be new processors that could detect signed integer
overflow (still for security reasons). On the other side, compilers are
smarter and smarter for optimizations, and code with UB can affect the
whole generated code; for instance, a compiler may find out that some
branch is not possible due to UB and do not generate the corresponding
code as a consequence. This sometimes yields strange bugs, difficult to
analyze. Currently it is very unlikely that this affects Mutt here, but
one never knows.
> --I can't currently get to dev.mutt.org to look at Vincent's
> full patch, but I'm guessing that's all it does... but probably it
> just choses a different wrong date to display.
This is not my patch, but tamo's. Together with my suggestion of change of
the {{{TIME_T_MAX}}} macro (as discussed above), it is probably be OK,
though I haven't tried it yet and I don't know whether it solves all
issues. For the display (if UBsan is not used), one additional advantage
is that the computed dates for large years are guaranteed to be seen in
Mutt as the largest dates, which is what is expected and IMHO sufficient.
> But, so what? Just don't compile with the extra pedantic flags and
> this doesn't really matter.
These are not just pedantic flags. There might be integer overflows or
other forms of UB at other places in Mutt, which could really be security
issues, and it is better to abort than leaving the software behave in an
erratic way. A software should never prevent the user from choosing
security protections.
--
Ticket URL: <https://dev.mutt.org/trac/ticket/3880#comment:11>
Mutt <http://www.mutt.org/>
The Mutt mail user agent
