Merge branch 'jk/commit-dates-parsing-fix'
authorJunio C Hamano <gitster@pobox.com>
Fri, 14 Mar 2014 21:25:44 +0000 (14:25 -0700)
committerJunio C Hamano <gitster@pobox.com>
Fri, 14 Mar 2014 21:25:44 +0000 (14:25 -0700)
Tighten codepaths that parse timestamps in commit objects.

* jk/commit-dates-parsing-fix:
show_ident_date: fix tz range check
log: do not segfault on gmtime errors
log: handle integer overflow in timestamps
date: check date overflow against time_t
fsck: report integer overflow in author timestamps
t4212: test bogus timestamps with git-log

cache.h
date.c
fsck.c
pretty.c
t/t1450-fsck.sh
t/t4212-log-corrupt.sh
diff --git a/cache.h b/cache.h
index bb0097e929b1411458ea9194bf3d7c0c56fb2f5a..5ce6dfeddb7ba356b7c469fcf86d00ecef83be9c 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -961,6 +961,7 @@ void datestamp(char *buf, int bufsize);
 unsigned long approxidate_careful(const char *, int *);
 unsigned long approxidate_relative(const char *date, const struct timeval *now);
 enum date_mode parse_date_format(const char *format);
+int date_overflows(unsigned long date);
 
 #define IDENT_STRICT          1
 #define IDENT_NO_DATE         2
diff --git a/date.c b/date.c
index 83b4166344b31ea615603acaeb95a873f2c05077..e1a2cee5688a64555d7820e8e02e3e86839c9641 100644 (file)
--- a/date.c
+++ b/date.c
@@ -184,8 +184,10 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
                tz = local_tzoffset(time);
 
        tm = time_to_tm(time, tz);
-       if (!tm)
-               return NULL;
+       if (!tm) {
+               tm = time_to_tm(0, 0);
+               tz = 0;
+       }
 
        strbuf_reset(&timebuf);
        if (mode == DATE_SHORT)
@@ -1113,3 +1115,20 @@ unsigned long approxidate_careful(const char *date, int *error_ret)
        gettimeofday(&tv, NULL);
        return approxidate_str(date, &tv, error_ret);
 }
+
+int date_overflows(unsigned long t)
+{
+       time_t sys;
+
+       /* If we overflowed our unsigned long, that's bad... */
+       if (t == ULONG_MAX)
+               return 1;
+
+       /*
+        * ...but we also are going to feed the result to system
+        * functions that expect time_t, which is often "signed long".
+        * Make sure that we fit into time_t, as well.
+        */
+       sys = t;
+       return t != sys || (t < 1) != (sys < 1);
+}
diff --git a/fsck.c b/fsck.c
index 99c049767484288f273f036b57683df04c1ef0df..64bf279fd7e42da1921a65d725007c52c7ddc5fb 100644 (file)
--- a/fsck.c
+++ b/fsck.c
@@ -245,6 +245,8 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)
 
 static int fsck_ident(char **ident, struct object *obj, fsck_error error_func)
 {
+       char *end;
+
        if (**ident == '<')
                return error_func(obj, FSCK_ERROR, "invalid author/committer line - missing space before email");
        *ident += strcspn(*ident, "<>\n");
@@ -264,10 +266,11 @@ static int fsck_ident(char **ident, struct object *obj, fsck_error error_func)
        (*ident)++;
        if (**ident == '0' && (*ident)[1] != ' ')
                return error_func(obj, FSCK_ERROR, "invalid author/committer line - zero-padded date");
-       *ident += strspn(*ident, "0123456789");
-       if (**ident != ' ')
+       if (date_overflows(strtoul(*ident, &end, 10)))
+               return error_func(obj, FSCK_ERROR, "invalid author/committer line - date causes integer overflow");
+       if (end == *ident || *end != ' ')
                return error_func(obj, FSCK_ERROR, "invalid author/committer line - bad date");
-       (*ident)++;
+       *ident = end + 1;
        if ((**ident != '+' && **ident != '-') ||
            !isdigit((*ident)[1]) ||
            !isdigit((*ident)[2]) ||
@@ -287,9 +290,6 @@ static int fsck_commit(struct commit *commit, fsck_error error_func)
        int parents = 0;
        int err;
 
-       if (commit->date == ULONG_MAX)
-               return error_func(&commit->object, FSCK_ERROR, "invalid author/committer line");
-
        if (memcmp(buffer, "tree ", 5))
                return error_func(&commit->object, FSCK_ERROR, "invalid format - expected 'tree' line");
        if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n')
index 87db08bd749e74fef0b2372ab71ea98baf57b322..6e266ddf2749ab1b2388f72e3a1101e98fa656cc 100644 (file)
--- a/pretty.c
+++ b/pretty.c
@@ -397,12 +397,18 @@ static const char *show_ident_date(const struct ident_split *ident,
                                   enum date_mode mode)
 {
        unsigned long date = 0;
-       int tz = 0;
+       long tz = 0;
 
        if (ident->date_begin && ident->date_end)
                date = strtoul(ident->date_begin, NULL, 10);
-       if (ident->tz_begin && ident->tz_end)
-               tz = strtol(ident->tz_begin, NULL, 10);
+       if (date_overflows(date))
+               date = 0;
+       else {
+               if (ident->tz_begin && ident->tz_end)
+                       tz = strtol(ident->tz_begin, NULL, 10);
+               if (tz >= INT_MAX || tz <= INT_MIN)
+                       tz = 0;
+       }
        return show_date(date, tz, mode);
 }
 
index d730734fde8e4de69fdf2662915bc67342198fc8..8c739c96135fe942d7e76236eb34429be93937a8 100755 (executable)
@@ -142,6 +142,20 @@ test_expect_success '> in name is reported' '
        grep "error in commit $new" out
 '
 
+# date is 2^64 + 1
+test_expect_success 'integer overflow in timestamps is reported' '
+       git cat-file commit HEAD >basis &&
+       sed "s/^\\(author .*>\\) [0-9]*/\\1 18446744073709551617/" \
+               <basis >bad-timestamp &&
+       new=$(git hash-object -t commit -w --stdin <bad-timestamp) &&
+       test_when_finished "remove_object $new" &&
+       git update-ref refs/heads/bogus "$new" &&
+       test_when_finished "git update-ref -d refs/heads/bogus" &&
+       git fsck 2>out &&
+       cat out &&
+       grep "error in commit $new.*integer overflow" out
+'
+
 test_expect_success 'tag pointing to nonexistent' '
        cat >invalid-tag <<-\EOF &&
        object ffffffffffffffffffffffffffffffffffffffff
index 93c7c366cfeffa46cc72665f6ce676bd37c3c2fe..3fa171541a161e0a5549c316b7c85608eb2d93d6 100755 (executable)
@@ -44,4 +44,49 @@ test_expect_success 'git log --format with broken author email' '
        test_cmp expect.err actual.err
 '
 
+munge_author_date () {
+       git cat-file commit "$1" >commit.orig &&
+       sed "s/^\(author .*>\) [0-9]*/\1 $2/" <commit.orig >commit.munge &&
+       git hash-object -w -t commit commit.munge
+}
+
+test_expect_success 'unparsable dates produce sentinel value' '
+       commit=$(munge_author_date HEAD totally_bogus) &&
+       echo "Date:   Thu Jan 1 00:00:00 1970 +0000" >expect &&
+       git log -1 $commit >actual.full &&
+       grep Date <actual.full >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success 'unparsable dates produce sentinel value (%ad)' '
+       commit=$(munge_author_date HEAD totally_bogus) &&
+       echo >expect &&
+       git log -1 --format=%ad $commit >actual
+       test_cmp expect actual
+'
+
+# date is 2^64 + 1
+test_expect_success 'date parser recognizes integer overflow' '
+       commit=$(munge_author_date HEAD 18446744073709551617) &&
+       echo "Thu Jan 1 00:00:00 1970 +0000" >expect &&
+       git log -1 --format=%ad $commit >actual &&
+       test_cmp expect actual
+'
+
+# date is 2^64 - 2
+test_expect_success 'date parser recognizes time_t overflow' '
+       commit=$(munge_author_date HEAD 18446744073709551614) &&
+       echo "Thu Jan 1 00:00:00 1970 +0000" >expect &&
+       git log -1 --format=%ad $commit >actual &&
+       test_cmp expect actual
+'
+
+# date is within 2^63-1, but enough to choke glibc's gmtime
+test_expect_success 'absurdly far-in-future dates produce sentinel' '
+       commit=$(munge_author_date HEAD 999999999999999999) &&
+       echo "Thu Jan 1 00:00:00 1970 +0000" >expect &&
+       git log -1 --format=%ad $commit >actual &&
+       test_cmp expect actual
+'
+
 test_done