Commit f327372504f195780dfd7179835bc64cf04357f5
2020-02-25T20:58:09
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 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67
diff --git a/src/pack.c b/src/pack.c
index fcf64f5..b4dbcdb 100644
--- a/src/pack.c
+++ b/src/pack.c
@@ -490,6 +490,12 @@ int git_packfile_resolve_header(
base_offset = get_delta_base(p, &w_curs, &curpos, type, offset);
git_mwindow_close(&w_curs);
+
+ if (base_offset < 0) {
+ error = (int)base_offset;
+ return error;
+ }
+
if ((error = git_packfile_stream_open(&stream, p, curpos)) < 0)
return error;
error = git_delta_read_header_fromstream(&base_size, size_p, &stream);
@@ -508,8 +514,14 @@ int git_packfile_resolve_header(
return error;
if (type != GIT_OBJECT_OFS_DELTA && type != GIT_OBJECT_REF_DELTA)
break;
+
base_offset = get_delta_base(p, &w_curs, &curpos, type, base_offset);
git_mwindow_close(&w_curs);
+
+ if (base_offset < 0) {
+ error = (int)base_offset;
+ return error;
+ }
}
*type_p = type;
@@ -586,10 +598,6 @@ static int pack_dependency_chain(git_dependency_chain *chain_out,
base_offset = get_delta_base(p, &w_curs, &curpos, type, obj_offset);
git_mwindow_close(&w_curs);
- if (base_offset == 0) {
- error = packfile_error("delta offset is zero");
- goto on_error;
- }
if (base_offset < 0) { /* must actually be an error code */
error = (int)base_offset;
goto on_error;
@@ -909,12 +917,12 @@ off64_t get_delta_base(
return GIT_EBUFS;
unsigned_base_offset += 1;
if (!unsigned_base_offset || MSB(unsigned_base_offset, 7))
- return 0; /* overflow */
+ return packfile_error("overflow");
c = base_info[used++];
unsigned_base_offset = (unsigned_base_offset << 7) + (c & 127);
}
if (unsigned_base_offset == 0 || (size_t)delta_obj_offset <= unsigned_base_offset)
- return 0; /* out of bound */
+ return packfile_error("out of bounds");
base_offset = delta_obj_offset - unsigned_base_offset;
*curpos += used;
} else if (type == GIT_OBJECT_REF_DELTA) {
@@ -941,7 +949,7 @@ off64_t get_delta_base(
return packfile_error("base entry delta is not in the same pack");
*curpos += 20;
} else
- return 0;
+ return packfile_error("unknown object type");
return base_offset;
}