author | Lukas Fleischer <cgit@cryptocrack.de> | 2011-04-05 08:38:53 (UTC) |
---|---|---|
committer | Lars Hjemli <hjemli@gmail.com> | 2011-05-23 20:58:35 (UTC) |
commit | 9afc883297b0d0943e9b358d2299950f33e8e5ed (patch) (unidiff) | |
tree | 27e81428c0a6ad4bbdf5633fc95b946b4a631d30 | |
parent | a0bf375a1a9b74056a913f3687c6f5b42ad4acf6 (diff) | |
download | cgit-9afc883297b0d0943e9b358d2299950f33e8e5ed.zip cgit-9afc883297b0d0943e9b358d2299950f33e8e5ed.tar.gz cgit-9afc883297b0d0943e9b358d2299950f33e8e5ed.tar.bz2 |
Avoid null pointer dereference in cgit_print_diff().
When calling cgit_print_diff() with a bad new_rev and a NULL old_rev,
checking for new_rev's parent commit will result in a null pointer
dereference. Returning on an invalid commit before dereferencing fixes
this. Spotted with clang-analyzer.
Signed-off-by: Lukas Fleischer <cgit@cryptocrack.de>
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
-rw-r--r-- | ui-diff.c | 8 |
1 files changed, 6 insertions, 2 deletions
@@ -324,63 +324,67 @@ static void filepair_cb(struct diff_filepair *pair) | |||
324 | if (use_ssdiff) | 324 | if (use_ssdiff) |
325 | html("<tr><td colspan='4'>Binary files differ</td></tr>"); | 325 | html("<tr><td colspan='4'>Binary files differ</td></tr>"); |
326 | else | 326 | else |
327 | html("Binary files differ"); | 327 | html("Binary files differ"); |
328 | } | 328 | } |
329 | if (use_ssdiff) | 329 | if (use_ssdiff) |
330 | cgit_ssdiff_footer(); | 330 | cgit_ssdiff_footer(); |
331 | } | 331 | } |
332 | 332 | ||
333 | void cgit_print_diff(const char *new_rev, const char *old_rev, const char *prefix) | 333 | void cgit_print_diff(const char *new_rev, const char *old_rev, const char *prefix) |
334 | { | 334 | { |
335 | enum object_type type; | 335 | enum object_type type; |
336 | unsigned long size; | 336 | unsigned long size; |
337 | struct commit *commit, *commit2; | 337 | struct commit *commit, *commit2; |
338 | 338 | ||
339 | if (!new_rev) | 339 | if (!new_rev) |
340 | new_rev = ctx.qry.head; | 340 | new_rev = ctx.qry.head; |
341 | get_sha1(new_rev, new_rev_sha1); | 341 | get_sha1(new_rev, new_rev_sha1); |
342 | type = sha1_object_info(new_rev_sha1, &size); | 342 | type = sha1_object_info(new_rev_sha1, &size); |
343 | if (type == OBJ_BAD) { | 343 | if (type == OBJ_BAD) { |
344 | cgit_print_error(fmt("Bad object name: %s", new_rev)); | 344 | cgit_print_error(fmt("Bad object name: %s", new_rev)); |
345 | return; | 345 | return; |
346 | } | 346 | } |
347 | commit = lookup_commit_reference(new_rev_sha1); | 347 | commit = lookup_commit_reference(new_rev_sha1); |
348 | if (!commit || parse_commit(commit)) | 348 | if (!commit || parse_commit(commit)) { |
349 | cgit_print_error(fmt("Bad commit: %s", sha1_to_hex(new_rev_sha1))); | 349 | cgit_print_error(fmt("Bad commit: %s", sha1_to_hex(new_rev_sha1))); |
350 | return; | ||
351 | } | ||
350 | 352 | ||
351 | if (old_rev) | 353 | if (old_rev) |
352 | get_sha1(old_rev, old_rev_sha1); | 354 | get_sha1(old_rev, old_rev_sha1); |
353 | else if (commit->parents && commit->parents->item) | 355 | else if (commit->parents && commit->parents->item) |
354 | hashcpy(old_rev_sha1, commit->parents->item->object.sha1); | 356 | hashcpy(old_rev_sha1, commit->parents->item->object.sha1); |
355 | else | 357 | else |
356 | hashclr(old_rev_sha1); | 358 | hashclr(old_rev_sha1); |
357 | 359 | ||
358 | if (!is_null_sha1(old_rev_sha1)) { | 360 | if (!is_null_sha1(old_rev_sha1)) { |
359 | type = sha1_object_info(old_rev_sha1, &size); | 361 | type = sha1_object_info(old_rev_sha1, &size); |
360 | if (type == OBJ_BAD) { | 362 | if (type == OBJ_BAD) { |
361 | cgit_print_error(fmt("Bad object name: %s", sha1_to_hex(old_rev_sha1))); | 363 | cgit_print_error(fmt("Bad object name: %s", sha1_to_hex(old_rev_sha1))); |
362 | return; | 364 | return; |
363 | } | 365 | } |
364 | commit2 = lookup_commit_reference(old_rev_sha1); | 366 | commit2 = lookup_commit_reference(old_rev_sha1); |
365 | if (!commit2 || parse_commit(commit2)) | 367 | if (!commit2 || parse_commit(commit2)) { |
366 | cgit_print_error(fmt("Bad commit: %s", sha1_to_hex(old_rev_sha1))); | 368 | cgit_print_error(fmt("Bad commit: %s", sha1_to_hex(old_rev_sha1))); |
369 | return; | ||
370 | } | ||
367 | } | 371 | } |
368 | 372 | ||
369 | if ((ctx.qry.ssdiff && !ctx.cfg.ssdiff) || (!ctx.qry.ssdiff && ctx.cfg.ssdiff)) | 373 | if ((ctx.qry.ssdiff && !ctx.cfg.ssdiff) || (!ctx.qry.ssdiff && ctx.cfg.ssdiff)) |
370 | use_ssdiff = 1; | 374 | use_ssdiff = 1; |
371 | 375 | ||
372 | print_ssdiff_link(); | 376 | print_ssdiff_link(); |
373 | cgit_print_diffstat(old_rev_sha1, new_rev_sha1, prefix); | 377 | cgit_print_diffstat(old_rev_sha1, new_rev_sha1, prefix); |
374 | 378 | ||
375 | if (use_ssdiff) { | 379 | if (use_ssdiff) { |
376 | html("<table summary='ssdiff' class='ssdiff'>"); | 380 | html("<table summary='ssdiff' class='ssdiff'>"); |
377 | } else { | 381 | } else { |
378 | html("<table summary='diff' class='diff'>"); | 382 | html("<table summary='diff' class='diff'>"); |
379 | html("<tr><td>"); | 383 | html("<tr><td>"); |
380 | } | 384 | } |
381 | cgit_diff_tree(old_rev_sha1, new_rev_sha1, filepair_cb, prefix, | 385 | cgit_diff_tree(old_rev_sha1, new_rev_sha1, filepair_cb, prefix, |
382 | ctx.qry.ignorews); | 386 | ctx.qry.ignorews); |
383 | if (!use_ssdiff) | 387 | if (!use_ssdiff) |
384 | html("</td></tr>"); | 388 | html("</td></tr>"); |
385 | html("</table>"); | 389 | html("</table>"); |
386 | } | 390 | } |