Skip to content

Commit f327372

Browse files
committed
pack: Improve error handling for get_delta_base()
This change moves the responsibility of setting the error upon failures of get_delta_base() to get_delta_base() instead of its callers. That way, the caller chan always check if the return value is negative and mark the whole operation as an error instead of using garbage values, which can lead to crashes if the .pack files are malformed.
1 parent 1c7fb21 commit f327372

File tree

1 file changed

+15
-7
lines changed

1 file changed

+15
-7
lines changed

src/pack.c

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,12 @@ int git_packfile_resolve_header(
490490

491491
base_offset = get_delta_base(p, &w_curs, &curpos, type, offset);
492492
git_mwindow_close(&w_curs);
493+
494+
if (base_offset < 0) {
495+
error = (int)base_offset;
496+
return error;
497+
}
498+
493499
if ((error = git_packfile_stream_open(&stream, p, curpos)) < 0)
494500
return error;
495501
error = git_delta_read_header_fromstream(&base_size, size_p, &stream);
@@ -508,8 +514,14 @@ int git_packfile_resolve_header(
508514
return error;
509515
if (type != GIT_OBJECT_OFS_DELTA && type != GIT_OBJECT_REF_DELTA)
510516
break;
517+
511518
base_offset = get_delta_base(p, &w_curs, &curpos, type, base_offset);
512519
git_mwindow_close(&w_curs);
520+
521+
if (base_offset < 0) {
522+
error = (int)base_offset;
523+
return error;
524+
}
513525
}
514526
*type_p = type;
515527

@@ -586,10 +598,6 @@ static int pack_dependency_chain(git_dependency_chain *chain_out,
586598
base_offset = get_delta_base(p, &w_curs, &curpos, type, obj_offset);
587599
git_mwindow_close(&w_curs);
588600

589-
if (base_offset == 0) {
590-
error = packfile_error("delta offset is zero");
591-
goto on_error;
592-
}
593601
if (base_offset < 0) { /* must actually be an error code */
594602
error = (int)base_offset;
595603
goto on_error;
@@ -909,12 +917,12 @@ off64_t get_delta_base(
909917
return GIT_EBUFS;
910918
unsigned_base_offset += 1;
911919
if (!unsigned_base_offset || MSB(unsigned_base_offset, 7))
912-
return 0; /* overflow */
920+
return packfile_error("overflow");
913921
c = base_info[used++];
914922
unsigned_base_offset = (unsigned_base_offset << 7) + (c & 127);
915923
}
916924
if (unsigned_base_offset == 0 || (size_t)delta_obj_offset <= unsigned_base_offset)
917-
return 0; /* out of bound */
925+
return packfile_error("out of bounds");
918926
base_offset = delta_obj_offset - unsigned_base_offset;
919927
*curpos += used;
920928
} else if (type == GIT_OBJECT_REF_DELTA) {
@@ -941,7 +949,7 @@ off64_t get_delta_base(
941949
return packfile_error("base entry delta is not in the same pack");
942950
*curpos += 20;
943951
} else
944-
return 0;
952+
return packfile_error("unknown object type");
945953

946954
return base_offset;
947955
}

0 commit comments

Comments
 (0)