{"thread":{"id":"18802","subject":"bug: git-http-push fails to validate email address in LOCK request","startedAt":"2009-04-09T11:19:44Z","lastAt":"2009-04-09T22:25:37Z","messageCount":4,"participants":["Riku Voipio","Mike Hommey"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"110913","messageId":"20090409111944.GA22297@kos.to","threadId":"18802","inReplyTo":null,"subject":"bug: git-http-push fails to validate email address in LOCK request","fromName":"Riku Voipio","fromEmail":"riku.voipio@iki.fi","sentAt":"2009-04-09T11:19:44Z","receivedAt":"2009-04-09T11:19:44Z","isPatch":false,"sender":{"key":"riku.voipio@iki.fi","avatar":null},"body":"Hi,\n\nIf for whatever case EMAIL env variable or gitconfig email= setting\ncontains any special xml character <>%&, the lock request in push\nwill fail, with a completly useless error message:\n\ngit push foo upstream-push:master\nError: cannot lock existing info/refs\nerror: failed to push some refs to 'https://git.foo.org/projects/foo'\n\nusing the undocumented GIT_CURL_VERBOSE=1 env variable:\n\n-snip-\n> LOCK /projects/foo/info/refs HTTP/1.1\nAuthorization: Basic nothisstringisnottheonefromthereallog\nUser-Agent: git/1.6.2.1\nHost: git.foo.org\nAccept: */*\nTimeout: Second-600\nContent-Type: text/xml\nContent-Length: 225\nExpect: 100-continue\n\n< HTTP/1.1 100 Continue\n* The requested URL returned error: 400\n* Closing connection #0\nError: cannot lock existing info/refs\nerror: failed to push some refs to 'https://git.foo.org/projects/foo'\n-snip-\n\nHelpfully, the actual lock request is omitted.\n\nAfter finding someone from git hoster to grep the logs:\n\n[Tue Apr 07 14:13:24 2009] [error] [client 1.1.1.1] XML Parser\nError: XML parser error code: not well-formed (invalid token) (4)\n\nfrom http-push.c, we see that the lock request is:\n\n#define LOCK_REQUEST \"<?xml version=\\\"1.0\\\" encoding=\\\"utf-8\\\" ?>\\n<D:lockinfo xmlns:D=\\\"DAV:\\\">\\n<D:lockscope><D:exclusive/></D:lockscope>\\n<D:locktype><D:write/></D:locktype>\\n<D:owner>\\n<D:href>mailto:%s</D:href>\\n</D:owner>\\n</D:lockinfo>\"\n\nSo there is exactly one variable being set. Turns out the email\naddress in .gitconfig was set to '<riku.voipio@iki.fi>'. Yes,\na user error. This was copied from $EMAIL env variable, where\nusing strings such as 'foo bar <foo.bar@corp.com>' have been\na norm forever.\n\n\n-- \n\"rm -rf\" only sounds scary if you don't have backups\n"},{"id":"110914","messageId":"20090409120205.GA29046@glandium.org","threadId":"18802","inReplyTo":"20090409111944.GA22297@kos.to","subject":"Re: bug: git-http-push fails to validate email address in LOCK request","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-04-09T12:02:06Z","receivedAt":"2009-04-09T12:02:06Z","isPatch":false,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Thu, Apr 09, 2009 at 02:19:44PM +0300, Riku Voipio <riku.voipio@iki.fi> wrote:\n> from http-push.c, we see that the lock request is:\n> \n> #define LOCK_REQUEST \"<?xml version=\\\"1.0\\\" encoding=\\\"utf-8\\\" ?>\\n<D:lockinfo xmlns:D=\\\"DAV:\\\">\\n<D:lockscope><D:exclusive/></D:lockscope>\\n<D:locktype><D:write/></D:locktype>\\n<D:owner>\\n<D:href>mailto:%s</D:href>\\n</D:owner>\\n</D:lockinfo>\"\n> \n> So there is exactly one variable being set. Turns out the email\n> address in .gitconfig was set to '<riku.voipio@iki.fi>'. Yes,\n> a user error. This was copied from $EMAIL env variable, where\n> using strings such as 'foo bar <foo.bar@corp.com>' have been\n> a norm forever.\n\nThat's the problem with generating XML by concatenating strings without\nchecks :(\n\nMike\n"},{"id":"110962","messageId":"1239315804-22629-1-git-send-email-mh@glandium.org","threadId":"18802","inReplyTo":"20090409111944.GA22297@kos.to","subject":"[PATCH] Replace \",<,>,& with their respective XML entities in DAV requests","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-04-09T22:23:24Z","receivedAt":"2009-04-09T22:23:24Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"If the repo url or the user email contain XML special characters, the\nremote DAV server is likely to reject the LOCK requests because the XML\nis then malformed.\n\nSigned-off-by: Mike Hommey <mh@glandium.org>\n---\n http-push.c |   36 ++++++++++++++++++++++++++++++++++--\n 1 files changed, 34 insertions(+), 2 deletions(-)\n\nNote I haven't tested more than with a quick run of t5540-http-push.sh.\nAlso note that this doesn't solve possible problems with non UTF-8\ncharacters in both strings we escape here. (a DAV server may rightfully\nreject XML containing non UTF-8 characters, since this XML is supposed\nto be UTF-8 according to its prolog, and would thus be malformed)\n\ndiff --git a/http-push.c b/http-push.c\nindex feeb340..29e6b6b 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -186,6 +186,32 @@ enum dav_header_flag {\n \tDAV_HEADER_TIMEOUT = (1u << 2)\n };\n \n+static char *xml_entities(char *s)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\twhile (*s) {\n+\t\tsize_t len = strcspn(s, \"\\\"<>&\");\n+\t\tstrbuf_add(&buf, s, len);\n+\t\ts += len;\n+\t\tswitch (*s) {\n+\t\tcase '\"':\n+\t\t\tstrbuf_addstr(&buf, \"&quot;\");\n+\t\t\tbreak;\n+\t\tcase '<':\n+\t\t\tstrbuf_addstr(&buf, \"&lt;\");\n+\t\t\tbreak;\n+\t\tcase '>':\n+\t\t\tstrbuf_addstr(&buf, \"&gt;\");\n+\t\t\tbreak;\n+\t\tcase '&':\n+\t\t\tstrbuf_addstr(&buf, \"&amp;\");\n+\t\t\tbreak;\n+\t\t}\n+\t\ts++;\n+\t}\n+\treturn buf.buf;\n+}\n+\n static struct curl_slist *get_dav_token_headers(struct remote_lock *lock, enum dav_header_flag options)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -1225,6 +1251,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)\n \tstruct remote_lock *lock = NULL;\n \tstruct curl_slist *dav_headers = NULL;\n \tstruct xml_ctx ctx;\n+\tchar *escaped;\n \n \turl = xmalloc(strlen(repo->url) + strlen(path) + 1);\n \tsprintf(url, \"%s%s\", repo->url, path);\n@@ -1259,7 +1286,9 @@ static struct remote_lock *lock_remote(const char *path, long timeout)\n \t\tep = strchr(ep + 1, '/');\n \t}\n \n-\tstrbuf_addf(&out_buffer.buf, LOCK_REQUEST, git_default_email);\n+\tescaped = xml_entities(git_default_email);\n+\tstrbuf_addf(&out_buffer.buf, LOCK_REQUEST, escaped);\n+\tfree(escaped);\n \n \tsprintf(timeout_header, \"Timeout: Second-%ld\", timeout);\n \tdav_headers = curl_slist_append(dav_headers, timeout_header);\n@@ -1584,8 +1613,11 @@ static int locking_available(void)\n \tstruct curl_slist *dav_headers = NULL;\n \tstruct xml_ctx ctx;\n \tint lock_flags = 0;\n+\tchar *escaped;\n \n-\tstrbuf_addf(&out_buffer.buf, PROPFIND_SUPPORTEDLOCK_REQUEST, repo->url);\n+\tescaped = xml_entities(repo->url);\n+\tstrbuf_addf(&out_buffer.buf, PROPFIND_SUPPORTEDLOCK_REQUEST, escaped);\n+\tfree(escaped);\n \n \tdav_headers = curl_slist_append(dav_headers, \"Depth: 0\");\n \tdav_headers = curl_slist_append(dav_headers, \"Content-Type: text/xml\");\n-- \n1.6.2.1\n"},{"id":"110965","messageId":"1239315937-22920-1-git-send-email-mh@glandium.org","threadId":"18802","inReplyTo":"20090409111944.GA22297@kos.to","subject":"[PATCH v2] Replace \",<,>,& with their respective XML entities in DAV requests","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-04-09T22:25:37Z","receivedAt":"2009-04-09T22:25:37Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"If the repo url or the user email contain XML special characters, the\nremote DAV server is likely to reject the LOCK requests because the XML\nis then malformed.\n\nSigned-off-by: Mike Hommey <mh@glandium.org>\n---\n http-push.c |   36 ++++++++++++++++++++++++++++++++++--\n 1 files changed, 34 insertions(+), 2 deletions(-)\n\nGah, the previous one didn't have a fix squashed in...\n\ndiff --git a/http-push.c b/http-push.c\nindex feeb340..5138224 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -186,6 +186,32 @@ enum dav_header_flag {\n \tDAV_HEADER_TIMEOUT = (1u << 2)\n };\n \n+static char *xml_entities(char *s)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\twhile (*s) {\n+\t\tsize_t len = strcspn(s, \"\\\"<>&\");\n+\t\tstrbuf_add(&buf, s, len);\n+\t\ts += len;\n+\t\tswitch (*s) {\n+\t\tcase '\"':\n+\t\t\tstrbuf_addstr(&buf, \"&quot;\");\n+\t\t\tbreak;\n+\t\tcase '<':\n+\t\t\tstrbuf_addstr(&buf, \"&lt;\");\n+\t\t\tbreak;\n+\t\tcase '>':\n+\t\t\tstrbuf_addstr(&buf, \"&gt;\");\n+\t\t\tbreak;\n+\t\tcase '&':\n+\t\t\tstrbuf_addstr(&buf, \"&amp;\");\n+\t\t\tbreak;\n+\t\t}\n+\t\ts++;\n+\t}\n+\treturn strbuf_detach(&buf, NULL);\n+}\n+\n static struct curl_slist *get_dav_token_headers(struct remote_lock *lock, enum dav_header_flag options)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -1225,6 +1251,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)\n \tstruct remote_lock *lock = NULL;\n \tstruct curl_slist *dav_headers = NULL;\n \tstruct xml_ctx ctx;\n+\tchar *escaped;\n \n \turl = xmalloc(strlen(repo->url) + strlen(path) + 1);\n \tsprintf(url, \"%s%s\", repo->url, path);\n@@ -1259,7 +1286,9 @@ static struct remote_lock *lock_remote(const char *path, long timeout)\n \t\tep = strchr(ep + 1, '/');\n \t}\n \n-\tstrbuf_addf(&out_buffer.buf, LOCK_REQUEST, git_default_email);\n+\tescaped = xml_entities(git_default_email);\n+\tstrbuf_addf(&out_buffer.buf, LOCK_REQUEST, escaped);\n+\tfree(escaped);\n \n \tsprintf(timeout_header, \"Timeout: Second-%ld\", timeout);\n \tdav_headers = curl_slist_append(dav_headers, timeout_header);\n@@ -1584,8 +1613,11 @@ static int locking_available(void)\n \tstruct curl_slist *dav_headers = NULL;\n \tstruct xml_ctx ctx;\n \tint lock_flags = 0;\n+\tchar *escaped;\n \n-\tstrbuf_addf(&out_buffer.buf, PROPFIND_SUPPORTEDLOCK_REQUEST, repo->url);\n+\tescaped = xml_entities(repo->url);\n+\tstrbuf_addf(&out_buffer.buf, PROPFIND_SUPPORTEDLOCK_REQUEST, escaped);\n+\tfree(escaped);\n \n \tdav_headers = curl_slist_append(dav_headers, \"Depth: 0\");\n \tdav_headers = curl_slist_append(dav_headers, \"Content-Type: text/xml\");\n-- \n1.6.2.1\n"}]}