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

[PATCH 8/9] serve: reject bogus v2 "command=ls-refs=foo"

From
Jeff King <peff@peff.net>
Date
Sep 14, 2021, 15:37 UTC
Message-ID
<YUDBsdTeX8myV1vY@coredump.intra.peff.net>
In-Reply-To
<YUC/6n1hhUbMJiLw@coredump.intra.peff.net>

When we see a line from the client like "command=ls-refs", we parse everything after the equals sign as a capability, which we check against our capabilities table. If we don't recognize the command (e.g., "command=foo"), we'll reject it. But we use the same parser that checks for regular capabilities like "object-format=sha256". And so we'll accept "ls-refs=foo", even though everything after the equals is bogus, and simply ignored.

This isn't really hurting anything, but the request does violate the spec. Let's tighten it up to prevent any surprising behavior.

Signed-off-by: Jeff King <peff@peff.net>
---
 serve.c              |  2 +-
 t/t5701-git-serve.sh | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/serve.c b/serve.c
index baa0a17502..123abbaa83 100644
--- a/serve.c
+++ b/serve.c
@@ -220,7 +220,7 @@ static int parse_command(const char *key, struct protocol_capability **command)
 		if (*command)
 			die("command '%s' requested after already requesting command '%s'",
 			    out, (*command)->name);
-		if (!cmd || !cmd->advertise(the_repository, NULL) || !cmd->command)
+		if (!cmd || !cmd->advertise(the_repository, NULL) || !cmd->command || value)
 			die("invalid command '%s'", out);
 
 		*command = cmd;
diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh
index b095bfa0ac..ebb41657ab 100755
--- a/t/t5701-git-serve.sh
+++ b/t/t5701-git-serve.sh
@@ -72,6 +72,16 @@ test_expect_success 'request invalid command' '
 	test_i18ngrep "invalid command" err
 '
 
+test_expect_success 'requested command is command=value' '
+	test-tool pkt-line pack >in <<-\EOF &&
+	command=ls-refs=whatever
+	object-format=$(test_oid algo)
+	0000
+	EOF
+	test_must_fail test-tool serve-v2 --stateless-rpc 2>err <in &&
+	grep invalid.command.*ls-refs=whatever err
+'
+
 test_expect_success 'wrong object-format' '
 	test-tool pkt-line pack >in <<-EOF &&
 	command=fetch
-- 
2.33.0.887.g5b1f44e68d
Previous: Jeff KingNext: Taylor Blau
Message 25 of 77 in “reducing memory allocations for v2 servers”
  1. 0/9 reducing memory allocations for v2 serversJeff King, Sep 14, 2021
  2. 1/9 serve: rename is_command() to parse_command()Jeff King, Sep 14, 2021
  3. 2/9 serve: return capability "value" from get_capability()Jeff King, Sep 14, 2021
  4. 3/9 serve: add "receive" method for v2 capabilities tableJeff King, Sep 14, 2021
  5. 4/9 serve: provide "receive" function for object-format capabilityJeff King, Sep 14, 2021
  6. Martin ÅgrenSep 14, 2021
  7. 5/9 serve: provide "receive" function for session-id capabilityJeff King, Sep 14, 2021
  8. Taylor BlauSep 14, 2021
  9. Jeff KingSep 14, 2021
  10. Taylor BlauSep 14, 2021
  11. Martin ÅgrenSep 14, 2021
  12. Jeff KingSep 14, 2021
  13. 6/9 serve: drop "keys" strvecJeff King, Sep 14, 2021
  14. Taylor BlauSep 14, 2021
  15. Jeff KingSep 14, 2021
  16. 7/9 ls-refs: ignore very long ref-prefix countsJeff King, Sep 14, 2021
  17. Taylor BlauSep 14, 2021
  18. Jeff KingSep 14, 2021
  19. Jeff KingSep 14, 2021
  20. Martin ÅgrenSep 14, 2021
  21. Jeff KingSep 14, 2021
  22. Jeff KingSep 14, 2021
  23. Taylor BlauSep 14, 2021
  24. Jeff KingSep 14, 2021
  25. 8/9 serve: reject bogus v2 "command=ls-refs=foo"Jeff King, Sep 14, 2021
  26. Taylor BlauSep 14, 2021
  27. 9/9 serve: reject commands used as capabilitiesJeff King, Sep 14, 2021
  28. Taylor BlauSep 14, 2021
  29. Junio C HamanoSep 14, 2021
  30. Jeff KingSep 14, 2021
  31. 0/11 limit memory allocations for v2 serversJeff King, Sep 14, 2021
  32. 01/11 serve: rename is_command() to parse_command()Jeff King, Sep 14, 2021
  33. 02/11 serve: return capability "value" from get_capability()Jeff King, Sep 14, 2021
  34. 03/11 serve: add "receive" method for v2 capabilities tableJeff King, Sep 14, 2021
  35. Ævar Arnfjörð BjarmasonSep 15, 2021
  36. Jeff KingSep 15, 2021
  37. Junio C HamanoSep 15, 2021
  38. Jeff KingSep 15, 2021
  39. 04/11 serve: provide "receive" function for object-format capabilityJeff King, Sep 14, 2021
  40. Junio C HamanoSep 15, 2021
  41. 05/11 serve: provide "receive" function for session-id capabilityJeff King, Sep 14, 2021
  42. Junio C HamanoSep 15, 2021
  43. 07/11 ls-refs: ignore very long ref-prefix countsJeff King, Sep 14, 2021
  44. Taylor BlauSep 15, 2021
  45. Jeff KingSep 15, 2021
  46. Eric SunshineSep 15, 2021
  47. Jeff KingSep 15, 2021
  48. 06/11 serve: drop "keys" strvecJeff King, Sep 14, 2021
  49. Junio C HamanoSep 15, 2021
  50. 08/11 docs/protocol-v2: clarify some ls-refs ref-prefix detailsJeff King, Sep 14, 2021
  51. 09/11 serve: reject bogus v2 "command=ls-refs=foo"Jeff King, Sep 14, 2021
  52. Ævar Arnfjörð BjarmasonSep 15, 2021
  53. Jeff KingSep 15, 2021
  54. Eric SunshineSep 15, 2021
  55. Jeff KingSep 15, 2021
  56. Junio C HamanoSep 15, 2021
  57. Jeff KingSep 15, 2021
  58. 10/11 serve: reject commands used as capabilitiesJeff King, Sep 14, 2021
  59. 11/11 ls-refs: reject unknown argumentsJeff King, Sep 14, 2021
  60. Ævar Arnfjörð BjarmasonSep 15, 2021
  61. Jeff KingSep 15, 2021
  62. Taylor BlauSep 15, 2021
  63. Jeff KingSep 15, 2021
  64. 0/11 limit memory allocations for v2 serversJeff King, Sep 15, 2021
  65. 01/11 serve: rename is_command() to parse_command()Jeff King, Sep 15, 2021
  66. 02/11 serve: return capability "value" from get_capability()Jeff King, Sep 15, 2021
  67. 03/11 serve: add "receive" method for v2 capabilities tableJeff King, Sep 15, 2021
  68. 04/11 serve: provide "receive" function for object-format capabilityJeff King, Sep 15, 2021
  69. 05/11 serve: provide "receive" function for session-id capabilityJeff King, Sep 15, 2021
  70. 06/11 serve: drop "keys" strvecJeff King, Sep 15, 2021
  71. 07/11 ls-refs: ignore very long ref-prefix countsJeff King, Sep 15, 2021
  72. 08/11 docs/protocol-v2: clarify some ls-refs ref-prefix detailsJeff King, Sep 15, 2021
  73. 09/11 serve: reject bogus v2 "command=ls-refs=foo"Jeff King, Sep 15, 2021
  74. 10/11 serve: reject commands used as capabilitiesJeff King, Sep 15, 2021
  75. 11/11 ls-refs: reject unknown argumentsJeff King, Sep 15, 2021
  76. Ævar Arnfjörð BjarmasonSep 15, 2021
  77. Jeff KingSep 15, 2021

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.