Bug 10643 - cmp_time() returns incorrect result due to integer overflow
Summary: cmp_time() returns incorrect result due to integer overflow
Status: RESOLVED FIXED
Alias: None
Product: rsync
Classification: Unclassified
Component: core (show other bugs)
Version: 3.1.1
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Wayne Davison
QA Contact: Rsync QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-31 01:38 UTC by William Hooper
Modified: 2014-06-16 01:02 UTC (History)
0 users

See Also:


Attachments
Patch to change cmp_time() to use difftime(3) (574 bytes, patch)
2014-05-31 01:38 UTC, William Hooper
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description William Hooper 2014-05-31 01:38:26 UTC
Created attachment 9998 [details]
Patch to change cmp_time() to use difftime(3)

In rsync-3.1.1pre1, if time_t is a signed integer, the cmp_time() function in util.c returns an incorrect result if one of its arguments is far in the past and the other is close to the present or in the future. This causes rsync to treat the times as equal when they actually differ by many years.

For example, if time_t is a 32-bit signed integer, when cmp_time() is called with the following times (in either order):

   1401411396   (May 30 00:56:36 2014)
  -2146953600   (Dec 20 00:00:00 1901)

it returns 0, indicating that the times should be treated as the same, not the expected -1 or +1, indicating that the times are different.

This behavior is apparently caused by integer overflows in the subtraction operations in the conditional expressions on lines 1322 and 1326 of util.c. The difference between two signed integer values can be greater than can be represented in a signed integer of the same size.

Attached is a patch to change cmp_time() to use the difftime() library function, available in C89 and later, to compute the difference between the time_t values.
Comment 1 Wayne Davison 2014-06-16 01:02:30 UTC
To avoid having to figure out if a system has difftime(), I just fixed the logic in cmp_time() to work even if things overflow.  Thanks for noticing the wacko result and diagnosing the cause.