From 8c7b68a8bf4ae3ad460c99e412360feb5bf8a43f Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 31 Aug 2026 17:25:41 +0000 Subject: [PATCH] trace2: remove use of xstrfmt() We continue removing the possibility of a die() in the trace2 API by banning xstrfmt(), which calls die() during a failure to format. Instead of allowing a die(), perform a soft failure by failing to output the trace2 data when such a failure occurs. This requires carefully concatenating strings using memcpy() to construct redacted data to avoid copying password information in traced URLs. Update t0212 to more carefully test this behavior to explicitly include the ":" string in the appropriate context. Helped-by: Elijah Newren Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- banned-die.h | 3 +++ t/t0212-trace2-event.sh | 12 ++++++++---- trace2.c | 34 ++++++++++++++++++++++++++++++++-- 3 files changed, 43 insertions(+), 6 deletions(-) diff --git a/banned-die.h b/banned-die.h index 0ad9a6c492..4d1800353d 100644 --- a/banned-die.h +++ b/banned-die.h @@ -17,6 +17,9 @@ #undef xstrdup #define xstrdup(str) BANNED(xstrdup) +#undef xstrfmt +#define xstrfmt(...) BANNED(xstrfmt) + #undef ALLOC_ARRAY #define ALLOC_ARRAY(x, alloc) BANNED(ALLOC_ARRAY) diff --git a/t/t0212-trace2-event.sh b/t/t0212-trace2-event.sh index f5358a1dd4..23df800395 100755 --- a/t/t0212-trace2-event.sh +++ b/t/t0212-trace2-event.sh @@ -332,7 +332,8 @@ test_expect_success 'unsafe URLs are redacted by default in cmd_start events' ' GIT_TRACE2_EVENT="$(pwd)/trace.event" \ test-tool trace2 300redact_start git clone https://user:pwd@example.com/ clone2 && - test_grep ! user:pwd trace.event + test_grep ! user:pwd trace.event && + test_grep "user:@example.com/" trace.event ' test_expect_success 'unsafe URLs are redacted by default in child_start events' ' @@ -341,7 +342,8 @@ test_expect_success 'unsafe URLs are redacted by default in child_start events' GIT_TRACE2_EVENT="$(pwd)/trace.event" \ test-tool trace2 301redact_child_start git clone https://user:pwd@example.com/ clone2 && - test_grep ! user:pwd trace.event + test_grep ! user:pwd trace.event && + test_grep "user:@example.com/" trace.event ' test_expect_success 'unsafe URLs are redacted by default in exec events' ' @@ -350,7 +352,8 @@ test_expect_success 'unsafe URLs are redacted by default in exec events' ' GIT_TRACE2_EVENT="$(pwd)/trace.event" \ test-tool trace2 302redact_exec git clone https://user:pwd@example.com/ clone2 && - test_grep ! user:pwd trace.event + test_grep ! user:pwd trace.event && + test_grep "user:@example.com/" trace.event ' test_expect_success 'unsafe URLs are redacted by default in def_param events' ' @@ -359,7 +362,8 @@ test_expect_success 'unsafe URLs are redacted by default in def_param events' ' GIT_TRACE2_EVENT="$(pwd)/trace.event" \ test-tool trace2 303redact_def_param url https://user:pwd@example.com/ && - test_grep ! user:pwd trace.event + test_grep ! user:pwd trace.event && + test_grep "user:@example.com/" trace.event ' test_done diff --git a/trace2.c b/trace2.c index ea021c602e..4a597d8213 100644 --- a/trace2.c +++ b/trace2.c @@ -261,7 +261,10 @@ int trace2_is_enabled(void) static const char *redact_arg(const char *arg) { const char *p, *colon; + const char *redact = ":"; + char *redacted; size_t at; + size_t prefix_len, suffix_len, redacted_len, redact_len; if (!trace2_redact || (!skip_prefix(arg, "https://", &p) && @@ -276,7 +279,25 @@ static const char *redact_arg(const char *arg) if (!colon) return arg; - return xstrfmt("%.*s:%s", (int)(colon - arg), arg, p + at); + redact_len = strlen(redact); + prefix_len = colon - arg; + suffix_len = strlen(p + at); + + if (unsigned_add_overflows(prefix_len, suffix_len) || + unsigned_add_overflows(prefix_len + suffix_len, redact_len) || + unsigned_add_overflows(prefix_len + suffix_len + redact_len, 1)) + return NULL; + + redacted_len = prefix_len + suffix_len + redact_len + 1; + + redacted = malloc(redacted_len); + if (!redacted) + return NULL; + + memcpy(redacted, arg, prefix_len); + memcpy(redacted + prefix_len, redact, redact_len); + memcpy(redacted + prefix_len + redact_len, p + at, suffix_len + 1); + return redacted; } /* @@ -301,6 +322,8 @@ static const char **redact_argv(const char **argv) if (!argv[i]) return argv; + if (!redacted) + return NULL; for (j = 0; argv[j]; j++) ; /* keep counting */ @@ -317,7 +340,14 @@ static const char **redact_argv(const char **argv) ret[i] = redacted; for (++i; argv[i]; i++) { redacted = redact_arg(argv[i]); - ret[i] = redacted ? redacted : argv[i]; + if (!redacted) { + for (j = 0; j < i; j++) + if (ret[j] != argv[j]) + free((void *)ret[j]); + free(ret); + return NULL; + } + ret[i] = redacted; } return ret;