git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Kristoffer Haugsbakk <code@khaugsbakk.name>
Date
Feb 13, 2024, 16:01 UTC
Message-ID
<cover.1707839454.git.code@khaugsbakk.name>
In-Reply-To
<1c959378cf495d7a3d70d0c7bdf08cc501ed6e5d.1707679627.git.code@khaugsbakk.name>

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 <tiago@pascoal.net>
Cc: Chris Torek <chris.torek@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>
Cc: Rubén Justo <rjusto@gmail.com>
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 <tiago@pascoal.net>
    -    Helped-by: Junio C Hamano <gitster@pobox.com>
         Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
     
    -
    - ## 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
Previous: Rubén JustoNext: Kristoffer Haugsbakk
Message 18 of 32 in “git column fails (or crashes) if padding is negative”
  1. Tiago PascoalFeb 9, 2024
  2. Kristoffer HaugsbakkFeb 9, 2024
  3. Junio C HamanoFeb 9, 2024
  4. Kristoffer HaugsbakkFeb 11, 2024
  5. Junio C HamanoFeb 12, 2024
  6. column: disallow negative paddingKristoffer Haugsbakk, Feb 9, 2024
  7. Kristoffer HaugsbakkFeb 9, 2024
  8. Chris TorekFeb 10, 2024
  9. Kristoffer HaugsbakkFeb 11, 2024
  10. Junio C HamanoFeb 11, 2024
  11. Kristoffer HaugsbakkFeb 11, 2024
  12. column: disallow negative paddingKristoffer Haugsbakk, Feb 11, 2024
  13. Rubén JustoFeb 11, 2024
  14. Rubén JustoFeb 11, 2024
  15. Kristoffer HaugsbakkFeb 12, 2024
  16. Kristoffer HaugsbakkFeb 12, 2024
  17. Rubén JustoFeb 12, 2024
  18. 0/2 column: disallow negative paddingKristoffer Haugsbakk, Feb 13, 2024
  19. 1/2 column: disallow negative paddingKristoffer Haugsbakk, Feb 13, 2024
  20. 2/2 column: guard against negative paddingKristoffer Haugsbakk, Feb 13, 2024
  21. Junio C HamanoFeb 13, 2024
  22. Rubén JustoFeb 13, 2024
  23. Junio C HamanoFeb 13, 2024
  24. Rubén JustoFeb 13, 2024
  25. Kristoffer HaugsbakkFeb 13, 2024
  26. Junio C HamanoFeb 13, 2024
  27. Rubén JustoFeb 13, 2024
  28. tag: error when git-column failsRubén Justo, Feb 13, 2024
  29. Junio C HamanoFeb 14, 2024
  30. Rubén JustoFeb 13, 2024
  31. Kristoffer HaugsbakkFeb 13, 2024
  32. Junio C HamanoFeb 13, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.