{"thread":{"id":"30318","subject":"Use git-config to Break your Config File","startedAt":"2012-04-24T02:57:19Z","lastAt":"2012-04-26T04:19:32Z","messageCount":3,"participants":["David E. Wheeler","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"189931","messageId":"9E01F5E2-FB51-418D-B50C-EB6DC63A4F84@justatheory.com","threadId":"30318","inReplyTo":null,"subject":"Use git-config to Break your Config File","fromName":"David E. Wheeler","fromEmail":"david@justatheory.com","sentAt":"2012-04-24T02:57:19Z","receivedAt":"2012-04-24T02:57:19Z","isPatch":false,"sender":{"key":"david@justatheory.com","avatar":"https://gravatar.com/avatar/17283ea1e4276cc8e69df64f9c5c731fa97d480f97694231f43384e186bdcd90?d=mp&s=160"},"body":"Gitsters,\n\nI was fooling around and realized that I could assign an empty string as the name of a config section, like so:\n\n    $ git config --rename-section my ''\n\nThis leaves the config file looking like this:\n\n    []\n        foo = goodbye\n\nWhich does not appear to be valid:\n\n    $ git config --get '.foo'          \n    fatal: bad config file line 13 in .git/config\n\nSo maybe git-config should disallow setting a section to an empty string?\n\nBest,\n\nDavid\n"},{"id":"190091","messageId":"20120426014714.GC8590@sigill.intra.peff.net","threadId":"30318","inReplyTo":"9E01F5E2-FB51-418D-B50C-EB6DC63A4F84@justatheory.com","subject":"[PATCH] config: reject bogus section names for --rename-section","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-04-26T01:47:14Z","receivedAt":"2012-04-26T01:47:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"You can feed junk to \"git config --rename-section\", which\nwill result in a config file that git will not even parse\n(so you cannot fix it with git-config). We already have\nsyntactic sanity checks when setting a variable; let's do\nthe same for section names.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nOn Mon, Apr 23, 2012 at 07:57:19PM -0700, David E. Wheeler wrote:\n\n> I was fooling around and realized that I could assign an empty string\n> as the name of a config section, like so:\n> \n>     $ git config --rename-section my ''\n> \n> This leaves the config file looking like this:\n> \n>     []\n>         foo = goodbye\n> \n> Which does not appear to be valid:\n> \n>     $ git config --get '.foo'          \n>     fatal: bad config file line 13 in .git/config\n> \n> So maybe git-config should disallow setting a section to an empty string?\n\nYes, we should definitely be more careful.\n\n config.c               |   24 +++++++++++++++++++++++-\n t/t1300-repo-config.sh |    8 ++++++++\n 2 files changed, 31 insertions(+), 1 deletion(-)\n\ndiff --git a/config.c b/config.c\nindex 68d3294..9ef947e 100644\n--- a/config.c\n+++ b/config.c\n@@ -1552,20 +1552,42 @@ static int section_name_match (const char *buf, const char *name)\n \treturn 0;\n }\n \n+static int section_name_is_ok(const char *name)\n+{\n+\t/* Empty section names are bogus. */\n+\tif (!*name)\n+\t\treturn 0;\n+\n+\t/*\n+\t * Before a dot, we must be alphanumeric or dash. After the first dot,\n+\t * anything goes, so we can stop checking.\n+\t */\n+\tfor (; *name && *name != '.'; name++)\n+\t\tif (*name != '-' && !isalnum(*name))\n+\t\t\treturn 0;\n+\treturn 1;\n+}\n+\n /* if new_name == NULL, the section is removed instead */\n int git_config_rename_section_in_file(const char *config_filename,\n \t\t\t\t      const char *old_name, const char *new_name)\n {\n \tint ret = 0, remove = 0;\n \tchar *filename_buf = NULL;\n-\tstruct lock_file *lock = xcalloc(sizeof(struct lock_file), 1);\n+\tstruct lock_file *lock;\n \tint out_fd;\n \tchar buf[1024];\n \tFILE *config_file;\n \n+\tif (new_name && !section_name_is_ok(new_name)) {\n+\t\tret = error(\"invalid section name: %s\", new_name);\n+\t\tgoto out;\n+\t}\n+\n \tif (!config_filename)\n \t\tconfig_filename = filename_buf = git_pathdup(\"config\");\n \n+\tlock = xcalloc(sizeof(struct lock_file), 1);\n \tout_fd = hold_lock_file_for_update(lock, config_filename, 0);\n \tif (out_fd < 0) {\n \t\tret = error(\"could not lock config file %s\", config_filename);\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 36e227b..a477453 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -550,6 +550,14 @@ EOF\n \n test_expect_success \"rename succeeded\" \"test_cmp expect .git/config\"\n \n+test_expect_success 'renaming empty section name is rejected' '\n+\ttest_must_fail git config --rename-section branch.zwei \"\"\n+'\n+\n+test_expect_success 'renaming to bogus section is rejected' '\n+\ttest_must_fail git config --rename-section branch.zwei \"bogus name\"\n+'\n+\n cat >> .git/config << EOF\n   [branch \"zwei\"] a = 1 [branch \"vier\"]\n EOF\n-- \n1.7.9.6.11.gd9951\n"},{"id":"190093","messageId":"xmqq62cnazvv.fsf@junio.mtv.corp.google.com","threadId":"30318","inReplyTo":"20120426014714.GC8590@sigill.intra.peff.net","subject":"Re: [PATCH] config: reject bogus section names for --rename-section","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-26T04:19:32Z","receivedAt":"2012-04-26T04:19:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks, will queue.\n"}]}