# bug: git-http-push fails to validate email address in LOCK request 4 messages from 2009-04-09 to 2009-04-09. Participants: Riku Voipio, Mike Hommey. Thread: https://gitlist.dev/t/18802 ## Riku Voipio, 2009-04-09 11:19 Subject: bug: git-http-push fails to validate email address in LOCK request Message-ID: <20090409111944.GA22297@kos.to> URL: https://gitlist.dev/e/20090409111944.GA22297%40kos.to ``` Hi, If for whatever case EMAIL env variable or gitconfig email= setting contains any special xml character <>%&, the lock request in push will fail, with a completly useless error message: git push foo upstream-push:master Error: cannot lock existing info/refs error: failed to push some refs to 'https://git.foo.org/projects/foo' using the undocumented GIT_CURL_VERBOSE=1 env variable: -snip- > LOCK /projects/foo/info/refs HTTP/1.1 Authorization: Basic nothisstringisnottheonefromthereallog User-Agent: git/1.6.2.1 Host: git.foo.org Accept: */* Timeout: Second-600 Content-Type: text/xml Content-Length: 225 Expect: 100-continue < HTTP/1.1 100 Continue * The requested URL returned error: 400 * Closing connection #0 Error: cannot lock existing info/refs error: failed to push some refs to 'https://git.foo.org/projects/foo' -snip- Helpfully, the actual lock request is omitted. After finding someone from git hoster to grep the logs: [Tue Apr 07 14:13:24 2009] [error] [client 1.1.1.1] XML Parser Error: XML parser error code: not well-formed (invalid token) (4) from http-push.c, we see that the lock request is: #define LOCK_REQUEST "\n\n\n\n\nmailto:%s\n\n" So there is exactly one variable being set. Turns out the email address in .gitconfig was set to ''. Yes, a user error. This was copied from $EMAIL env variable, where using strings such as 'foo bar ' have been a norm forever. -- "rm -rf" only sounds scary if you don't have backups ``` ## Mike Hommey, 2009-04-09 12:02 Subject: Re: bug: git-http-push fails to validate email address in LOCK request Message-ID: <20090409120205.GA29046@glandium.org> URL: https://gitlist.dev/e/20090409120205.GA29046%40glandium.org In-Reply-To: <20090409111944.GA22297@kos.to> ``` On Thu, Apr 09, 2009 at 02:19:44PM +0300, Riku Voipio wrote: > from http-push.c, we see that the lock request is: > > #define LOCK_REQUEST "\n\n\n\n\nmailto:%s\n\n" > > So there is exactly one variable being set. Turns out the email > address in .gitconfig was set to ''. Yes, > a user error. This was copied from $EMAIL env variable, where > using strings such as 'foo bar ' have been > a norm forever. That's the problem with generating XML by concatenating strings without checks :( Mike ``` ## Mike Hommey, 2009-04-09 22:23 Subject: [PATCH] Replace ",<,>,& with their respective XML entities in DAV requests Message-ID: <1239315804-22629-1-git-send-email-mh@glandium.org> URL: https://gitlist.dev/e/1239315804-22629-1-git-send-email-mh%40glandium.org In-Reply-To: <20090409111944.GA22297@kos.to> ``` If the repo url or the user email contain XML special characters, the remote DAV server is likely to reject the LOCK requests because the XML is then malformed. Signed-off-by: Mike Hommey --- http-push.c | 36 ++++++++++++++++++++++++++++++++++-- 1 files changed, 34 insertions(+), 2 deletions(-) Note I haven't tested more than with a quick run of t5540-http-push.sh. Also note that this doesn't solve possible problems with non UTF-8 characters in both strings we escape here. (a DAV server may rightfully reject XML containing non UTF-8 characters, since this XML is supposed to be UTF-8 according to its prolog, and would thus be malformed) diff --git a/http-push.c b/http-push.c index feeb340..29e6b6b 100644 --- a/http-push.c +++ b/http-push.c @@ -186,6 +186,32 @@ enum dav_header_flag { DAV_HEADER_TIMEOUT = (1u << 2) }; +static char *xml_entities(char *s) +{ + struct strbuf buf = STRBUF_INIT; + while (*s) { + size_t len = strcspn(s, "\"<>&"); + strbuf_add(&buf, s, len); + s += len; + switch (*s) { + case '"': + strbuf_addstr(&buf, """); + break; + case '<': + strbuf_addstr(&buf, "<"); + break; + case '>': + strbuf_addstr(&buf, ">"); + break; + case '&': + strbuf_addstr(&buf, "&"); + break; + } + s++; + } + return buf.buf; +} + static struct curl_slist *get_dav_token_headers(struct remote_lock *lock, enum dav_header_flag options) { struct strbuf buf = STRBUF_INIT; @@ -1225,6 +1251,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout) struct remote_lock *lock = NULL; struct curl_slist *dav_headers = NULL; struct xml_ctx ctx; + char *escaped; url = xmalloc(strlen(repo->url) + strlen(path) + 1); sprintf(url, "%s%s", repo->url, path); @@ -1259,7 +1286,9 @@ static struct remote_lock *lock_remote(const char *path, long timeout) ep = strchr(ep + 1, '/'); } - strbuf_addf(&out_buffer.buf, LOCK_REQUEST, git_default_email); + escaped = xml_entities(git_default_email); + strbuf_addf(&out_buffer.buf, LOCK_REQUEST, escaped); + free(escaped); sprintf(timeout_header, "Timeout: Second-%ld", timeout); dav_headers = curl_slist_append(dav_headers, timeout_header); @@ -1584,8 +1613,11 @@ static int locking_available(void) struct curl_slist *dav_headers = NULL; struct xml_ctx ctx; int lock_flags = 0; + char *escaped; - strbuf_addf(&out_buffer.buf, PROPFIND_SUPPORTEDLOCK_REQUEST, repo->url); + escaped = xml_entities(repo->url); + strbuf_addf(&out_buffer.buf, PROPFIND_SUPPORTEDLOCK_REQUEST, escaped); + free(escaped); dav_headers = curl_slist_append(dav_headers, "Depth: 0"); dav_headers = curl_slist_append(dav_headers, "Content-Type: text/xml"); -- 1.6.2.1 ``` ## Mike Hommey, 2009-04-09 22:25 Subject: [PATCH v2] Replace ",<,>,& with their respective XML entities in DAV requests Message-ID: <1239315937-22920-1-git-send-email-mh@glandium.org> URL: https://gitlist.dev/e/1239315937-22920-1-git-send-email-mh%40glandium.org In-Reply-To: <20090409111944.GA22297@kos.to> ``` If the repo url or the user email contain XML special characters, the remote DAV server is likely to reject the LOCK requests because the XML is then malformed. Signed-off-by: Mike Hommey --- http-push.c | 36 ++++++++++++++++++++++++++++++++++-- 1 files changed, 34 insertions(+), 2 deletions(-) Gah, the previous one didn't have a fix squashed in... diff --git a/http-push.c b/http-push.c index feeb340..5138224 100644 --- a/http-push.c +++ b/http-push.c @@ -186,6 +186,32 @@ enum dav_header_flag { DAV_HEADER_TIMEOUT = (1u << 2) }; +static char *xml_entities(char *s) +{ + struct strbuf buf = STRBUF_INIT; + while (*s) { + size_t len = strcspn(s, "\"<>&"); + strbuf_add(&buf, s, len); + s += len; + switch (*s) { + case '"': + strbuf_addstr(&buf, """); + break; + case '<': + strbuf_addstr(&buf, "<"); + break; + case '>': + strbuf_addstr(&buf, ">"); + break; + case '&': + strbuf_addstr(&buf, "&"); + break; + } + s++; + } + return strbuf_detach(&buf, NULL); +} + static struct curl_slist *get_dav_token_headers(struct remote_lock *lock, enum dav_header_flag options) { struct strbuf buf = STRBUF_INIT; @@ -1225,6 +1251,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout) struct remote_lock *lock = NULL; struct curl_slist *dav_headers = NULL; struct xml_ctx ctx; + char *escaped; url = xmalloc(strlen(repo->url) + strlen(path) + 1); sprintf(url, "%s%s", repo->url, path); @@ -1259,7 +1286,9 @@ static struct remote_lock *lock_remote(const char *path, long timeout) ep = strchr(ep + 1, '/'); } - strbuf_addf(&out_buffer.buf, LOCK_REQUEST, git_default_email); + escaped = xml_entities(git_default_email); + strbuf_addf(&out_buffer.buf, LOCK_REQUEST, escaped); + free(escaped); sprintf(timeout_header, "Timeout: Second-%ld", timeout); dav_headers = curl_slist_append(dav_headers, timeout_header); @@ -1584,8 +1613,11 @@ static int locking_available(void) struct curl_slist *dav_headers = NULL; struct xml_ctx ctx; int lock_flags = 0; + char *escaped; - strbuf_addf(&out_buffer.buf, PROPFIND_SUPPORTEDLOCK_REQUEST, repo->url); + escaped = xml_entities(repo->url); + strbuf_addf(&out_buffer.buf, PROPFIND_SUPPORTEDLOCK_REQUEST, escaped); + free(escaped); dav_headers = curl_slist_append(dav_headers, "Depth: 0"); dav_headers = curl_slist_append(dav_headers, "Content-Type: text/xml"); -- 1.6.2.1 ```