@matheusbsbs / matheus-bagliano / commits / d4b8de0420

fsck: report integer overflow in author timestamps

When we check commit objects, we complain if commit->date is ULONG_MAX, which is an indication that we saw integer overflow when parsing it. However, we do not do any check at all for author lines, which also contain a timestamp. Let's actually check the timestamps on each ident line with strtoul. This catches both author and committer lines, and we can get rid of the now-redundant commit->date check. Note that like the existing check, we compare only against ULONG_MAX. Now that we are calling strtoul at the site of the check, we could be slightly more careful and also check that errno is set to ERANGE. However, this will make further refactoring in future patches a little harder, and it doesn't really matter in practice. For 32-bit systems, one would have to create a commit at the exact wrong second in 2038. But by the time we get close to that, all systems will hopefully have moved to 64-bit (and if they haven't, they have a real problem one second later). For 64-bit systems, by the time we get close to ULONG_MAX, all systems will hopefully have been consumed in the fiery wrath of our expanding Sun. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Feb 24, 2014 at 02:39 UTC d4b8de0420ffcc7a654ddc6c69a96d3c1b25b4fa

2 files changed +20 -6

fsck.c

+6 -6

@@ -245,6 +245,8 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)

245

246 static int fsck_ident(char **ident, struct object *obj, fsck_error error_func)

247 {

248 + char *end;

249 +

250 if (**ident == '<')

251 return error_func(obj, FSCK_ERROR, "invalid author/committer line - missing space before email");

252 *ident += strcspn(*ident, "<>\n");

@@ -264,10 +266,11 @@ static int fsck_ident(char **ident, struct object *obj, fsck_error error_func)

266 (*ident)++;

267 if (**ident == '0' && (*ident)[1] != ' ')

268 return error_func(obj, FSCK_ERROR, "invalid author/committer line - zero-padded date");

267 - *ident += strspn(*ident, "0123456789");

268 - if (**ident != ' ')

269 + if (strtoul(*ident, &end, 10) == ULONG_MAX)

270 + return error_func(obj, FSCK_ERROR, "invalid author/committer line - date causes integer overflow");

271 + if (end == *ident || *end != ' ')

272 return error_func(obj, FSCK_ERROR, "invalid author/committer line - bad date");

270 - (*ident)++;

273 + *ident = end + 1;

274 if ((**ident != '+' && **ident != '-') ||

275 !isdigit((*ident)[1]) ||

276 !isdigit((*ident)[2]) ||

@@ -287,9 +290,6 @@ static int fsck_commit(struct commit *commit, fsck_error error_func)

290 int parents = 0;

291 int err;

292

290 - if (commit->date == ULONG_MAX)

291 - return error_func(&commit->object, FSCK_ERROR, "invalid author/committer line");

292 -

293 if (memcmp(buffer, "tree ", 5))

294 return error_func(&commit->object, FSCK_ERROR, "invalid format - expected 'tree' line");

295 if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n')

t/t1450-fsck.sh

+14

@@ -142,6 +142,20 @@ test_expect_success '> in name is reported' '

142 grep "error in commit $new" out

143 '

144

145 +# date is 2^64 + 1

146 +test_expect_success 'integer overflow in timestamps is reported' '

147 + git cat-file commit HEAD >basis &&

148 + sed "s/^\$author .*>\$ [0-9]*/\\1 18446744073709551617/" \

149 + <basis >bad-timestamp &&

150 + new=$(git hash-object -t commit -w --stdin <bad-timestamp) &&

151 + test_when_finished "remove_object $new" &&

152 + git update-ref refs/heads/bogus "$new" &&

153 + test_when_finished "git update-ref -d refs/heads/bogus" &&

154 + git fsck 2>out &&

155 + cat out &&

156 + grep "error in commit $new.*integer overflow" out

157 +'

158 +

159 test_expect_success 'tag pointing to nonexistent' '

160 cat >invalid-tag <<-\EOF &&

161 object ffffffffffffffffffffffffffffffffffffffff