Subject: [PATCH v3 0/2] column: disallow negative padding

From: Kristoffer Haugsbakk <[email-masked]>
Date: Tue, 13 Feb 2024 17:01:19 +0100

Fix bug in git-column(1): a user can pass a negative `padding` which
causes issues inside the memory allocator.

§ Changes in v3

Incorporate Ruben’s suggestion about guarding against negative padding
with `BUG` in `column.c` (not `builtin/column.c`). This then supersedes
Junio’s extra conditional checks since they are no longer needed. The
series gets split into two patches.

Cc: Tiago Pascoal <[email-masked]>
Cc: Chris Torek <[email-masked]>
Cc: Junio C Hamano <[email-masked]>
Cc: Rubén Justo <[email-masked]>

Kristoffer Haugsbakk (2):
  column: disallow negative padding
  column: guard against negative padding

 builtin/column.c  |  2 ++
 column.c          |  4 ++++
 t/t9002-column.sh | 11 +++++++++++
 3 files changed, 17 insertions(+)

Range-diff against v2:
1:  1c959378cf4 ! 1:  4cac42ca6f8 column: disallow negative padding
    @@ Commit message
         A negative padding does not make sense and can cause errors in the
         memory allocator since it’s interpreted as an unsigned integer.
     
    -    Disallow negative padding. Also guard against negative padding in
    -    `column.c` where it is conditionally used.
    -
         Reported-by: Tiago Pascoal <[email-masked]>
    -    Helped-by: Junio C Hamano <[email-masked]>
         Signed-off-by: Kristoffer Haugsbakk <[email-masked]>
     
    -
    - ## Notes (series) ##
    -    v2:
    -    • Incorporate Junio’s changes (guard against negative padding in
    -      `column.c`)
    -    • Tweak commit message based on Junio’s analysis
    -    • Use gettext for error message
    -      • However I noticed that the “translation string” from `fast-import`
    -        isn’t a translation string. So let’s invent a new one and use a
    -        parameter so that it can be used elsewhere.
    -    • Make a test
    -
      ## builtin/column.c ##
     @@ builtin/column.c: int cmd_column(int argc, const char **argv, const char *prefix)
      	memset(&copts, 0, sizeof(copts));
    @@ builtin/column.c: int cmd_column(int argc, const char **argv, const char *prefix
      		usage_with_options(builtin_column_usage, options);
      	if (real_command || command) {
     
    - ## column.c ##
    -@@ column.c: void print_columns(const struct string_list *list, unsigned int colopts,
    - 	memset(&nopts, 0, sizeof(nopts));
    - 	nopts.indent = opts && opts->indent ? opts->indent : "";
    - 	nopts.nl = opts && opts->nl ? opts->nl : "\n";
    --	nopts.padding = opts ? opts->padding : 1;
    -+	nopts.padding = (opts && 0 <= opts->padding) ? opts->padding : 1;
    - 	nopts.width = opts && opts->width ? opts->width : term_columns() - 1;
    - 	if (!column_active(colopts)) {
    - 		display_plain(list, "", "\n");
    -@@ column.c: int run_column_filter(int colopts, const struct column_options *opts)
    - 		strvec_pushf(argv, "--width=%d", opts->width);
    - 	if (opts && opts->indent)
    - 		strvec_pushf(argv, "--indent=%s", opts->indent);
    --	if (opts && opts->padding)
    -+	if (opts && 0 <= opts->padding)
    - 		strvec_pushf(argv, "--padding=%d", opts->padding);
    - 
    - 	fflush(stdout);
    -
      ## t/t9002-column.sh ##
     @@ t/t9002-column.sh: EOF
      	test_cmp expected actual
-:  ----------- > 2:  9355fc98e3d column: guard against negative padding
-- 
2.43.0