[PATCH v3 0/2] builtin/blame: fix out-of-bounds reads and writes
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Jan 10, 2025, 11:26 UTC
- Message-ID
- <20250110-b4-pks-blame-truncate-hash-length-v3-0-e61f25b68f30@pks.im>
- In-Reply-To
- <20250109-b4-pks-blame-truncate-hash-length-v1-1-9ad4bb09e059@pks.im>
Hi,
This fixes the issues reported in [1] and [2]. Thanks!
Changes in v2:
- Take into account that we may strip ^, * and ? indicators by moving
around the check.
- Fix the testcase so that it actually fails without the fix.
- Link to v1: https://lore.kernel.org/r/20250109-b4-pks-blame-truncate-hash-length-v1-1-9ad4bb09e059@pks.imChanges in v3:
- Add another testcase for boundary commits.
- Fix another out-of-bound write noticed by Coverity. This bug is not
a regression in v2.48.0, but is a preexisting error.
- Simplify the printf statement a bit by using a ternary statement.
- Link to v2: https://lore.kernel.org/r/20250109-b4-pks-blame-truncate-hash-length-v2-1-589c81a6ddb0@pks.imPatrick
[1]: <4d812802-afbc-4635-7a19-73896fcda625@gmx.de> [2]: <48ca0114-124b-e3f5-af80-1e302bf9ce52@gmx.de>
---
Patrick Steinhardt (2):
builtin/blame: fix out-of-bounds read with excessive `--abbrev`
builtin/blame: fix out-of-bounds write with blank boundary commitsbuiltin/blame.c | 9 +++++---- t/t8002-blame.sh | 26 ++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-)
Range-diff versus v2:
1: 7065637d8e ! 1: 3865bfe643 builtin/blame: fix out-of-bounds read with excessive `--abbrev`
@@ builtin/blame.c: static void emit_other(struct blame_scoreboard *sb, struct blam
}
- fwrite(hex, 1, length, stdout);
+
-+ if (length > GIT_MAX_HEXSZ)
-+ length = GIT_MAX_HEXSZ;
-+ printf("%.*s", (int)length, hex);
++ printf("%.*s", (int)(length < GIT_MAX_HEXSZ ? length : GIT_MAX_HEXSZ), hex);
if (opt & OUTPUT_ANNOTATE_COMPAT) {
const char *name;
if (opt & OUTPUT_SHOW_EMAIL)
@@ t/t8002-blame.sh: test_expect_success '--no-abbrev works like --abbrev with full
+test_expect_success 'blame --abbrev gets truncated' '
+ check_abbrev $hexsz --abbrev=9000 HEAD
+'
++
++test_expect_success 'blame --abbrev gets truncated with boundary commit' '
++ check_abbrev $hexsz --abbrev=9000 ^HEAD
++'
+
test_expect_success '--exclude-promisor-objects does not BUG-crash' '
test_must_fail git blame --exclude-promisor-objects one
-: ---------- > 2: af0af67a8a builtin/blame: fix out-of-bounds write with blank boundary commits--- base-commit: 14650065b76b28d3cfa9453356ac5669b19e706e change-id: 20250109-b4-pks-blame-truncate-hash-length-c875cac66d71