From 4656bf47fca857df51b5d6f4b7b052192b3b2317 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Thu, 31 Jan 2013 13:02:07 -0800 Subject: [PATCH] Verify Content-Type from smart HTTP servers Before parsing a suspected smart-HTTP response verify the returned Content-Type matches the standard. This protects a client from attempting to process a payload that smells like a smart-HTTP server response. JGit has been doing this check on all responses since the dawn of time. I mistakenly failed to include it in git-core when smart HTTP was introduced. At the time I didn't know how to get the Content-Type from libcurl. I punted, meant to circle back and fix this, and just plain forgot about it. Signed-off-by: Shawn Pearce Signed-off-by: Junio C Hamano --- http-push.c | 4 ++-- http.c | 31 ++++++++++++++++++++++--------- http.h | 2 +- remote-curl.c | 17 ++++++++++++----- t/lib-httpd.sh | 1 + t/lib-httpd/apache.conf | 4 ++++ t/lib-httpd/broken-smart-http.sh | 11 +++++++++++ t/t5551-http-fetch.sh | 6 ++++++ 8 files changed, 59 insertions(+), 17 deletions(-) create mode 100755 t/lib-httpd/broken-smart-http.sh diff --git a/http-push.c b/http-push.c index 8701c1215d..ba45b7b501 100644 --- a/http-push.c +++ b/http-push.c @@ -1560,7 +1560,7 @@ static int remote_exists(const char *path) sprintf(url, "%s%s", repo->url, path); - switch (http_get_strbuf(url, NULL, 0)) { + switch (http_get_strbuf(url, NULL, NULL, 0)) { case HTTP_OK: ret = 1; break; @@ -1584,7 +1584,7 @@ static void fetch_symref(const char *path, char **symref, unsigned char *sha1) url = xmalloc(strlen(repo->url) + strlen(path) + 1); sprintf(url, "%s%s", repo->url, path); - if (http_get_strbuf(url, &buffer, 0) != HTTP_OK) + if (http_get_strbuf(url, NULL, &buffer, 0) != HTTP_OK) die("Couldn't get %s for remote symref\n%s", url, curl_errorstr); free(url); diff --git a/http.c b/http.c index 44f35256e4..d868d8b3d9 100644 --- a/http.c +++ b/http.c @@ -788,7 +788,8 @@ int handle_curl_result(struct slot_results *results) #define HTTP_REQUEST_STRBUF 0 #define HTTP_REQUEST_FILE 1 -static int http_request(const char *url, void *result, int target, int options) +static int http_request(const char *url, struct strbuf *type, + void *result, int target, int options) { struct active_request_slot *slot; struct slot_results results; @@ -838,24 +839,36 @@ static int http_request(const char *url, void *result, int target, int options) ret = HTTP_START_FAILED; } + if (type) { + char *t; + curl_easy_getinfo(slot->curl, CURLINFO_CONTENT_TYPE, &t); + if (t) + strbuf_addstr(type, t); + } + curl_slist_free_all(headers); strbuf_release(&buf); return ret; } -static int http_request_reauth(const char *url, void *result, int target, +static int http_request_reauth(const char *url, + struct strbuf *type, + void *result, int target, int options) { - int ret = http_request(url, result, target, options); + int ret = http_request(url, type, result, target, options); if (ret != HTTP_REAUTH) return ret; - return http_request(url, result, target, options); + return http_request(url, type, result, target, options); } -int http_get_strbuf(const char *url, struct strbuf *result, int options) +int http_get_strbuf(const char *url, + struct strbuf *type, + struct strbuf *result, int options) { - return http_request_reauth(url, result, HTTP_REQUEST_STRBUF, options); + return http_request_reauth(url, type, result, + HTTP_REQUEST_STRBUF, options); } /* @@ -878,7 +891,7 @@ static int http_get_file(const char *url, const char *filename, int options) goto cleanup; } - ret = http_request_reauth(url, result, HTTP_REQUEST_FILE, options); + ret = http_request_reauth(url, NULL, result, HTTP_REQUEST_FILE, options); fclose(result); if ((ret == HTTP_OK) && move_temp_to_file(tmpfile.buf, filename)) @@ -904,7 +917,7 @@ int http_fetch_ref(const char *base, struct ref *ref) int ret = -1; url = quote_ref_url(base, ref->name); - if (http_get_strbuf(url, &buffer, HTTP_NO_CACHE) == HTTP_OK) { + if (http_get_strbuf(url, NULL, &buffer, HTTP_NO_CACHE) == HTTP_OK) { strbuf_rtrim(&buffer); if (buffer.len == 40) ret = get_sha1_hex(buffer.buf, ref->old_sha1); @@ -997,7 +1010,7 @@ int http_get_info_packs(const char *base_url, struct packed_git **packs_head) strbuf_addstr(&buf, "objects/info/packs"); url = strbuf_detach(&buf, NULL); - ret = http_get_strbuf(url, &buf, HTTP_NO_CACHE); + ret = http_get_strbuf(url, NULL, &buf, HTTP_NO_CACHE); if (ret != HTTP_OK) goto cleanup; diff --git a/http.h b/http.h index 0a80d303e3..25d1931398 100644 --- a/http.h +++ b/http.h @@ -132,7 +132,7 @@ extern char *get_remote_object_url(const char *url, const char *hex, * * If the result pointer is NULL, a HTTP HEAD request is made instead of GET. */ -int http_get_strbuf(const char *url, struct strbuf *result, int options); +int http_get_strbuf(const char *url, struct strbuf *content_type, struct strbuf *result, int options); /* * Prints an error message using error() containing url and curl_errorstr, diff --git a/remote-curl.c b/remote-curl.c index 9a8b123507..933c69ac26 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -92,6 +92,8 @@ static void free_discovery(struct discovery *d) static struct discovery* discover_refs(const char *service) { + struct strbuf exp = STRBUF_INIT; + struct strbuf type = STRBUF_INIT; struct strbuf buffer = STRBUF_INIT; struct discovery *last = last_discovery; char *refs_url; @@ -113,7 +115,7 @@ static struct discovery* discover_refs(const char *service) } refs_url = strbuf_detach(&buffer, NULL); - http_ret = http_get_strbuf(refs_url, &buffer, HTTP_NO_CACHE); + http_ret = http_get_strbuf(refs_url, &type, &buffer, HTTP_NO_CACHE); switch (http_ret) { case HTTP_OK: break; @@ -132,17 +134,20 @@ static struct discovery* discover_refs(const char *service) last->buf_alloc = strbuf_detach(&buffer, &last->len); last->buf = last->buf_alloc; - if (maybe_smart && 5 <= last->len && last->buf[4] == '#') { - /* smart HTTP response; validate that the service + strbuf_addf(&exp, "application/x-%s-advertisement", service); + if (maybe_smart && + (5 <= last->len && last->buf[4] == '#') && + !strbuf_cmp(&exp, &type)) { + /* + * smart HTTP response; validate that the service * pkt-line matches our request. */ - struct strbuf exp = STRBUF_INIT; - if (packet_get_line(&buffer, &last->buf, &last->len) <= 0) die("%s has invalid packet header", refs_url); if (buffer.len && buffer.buf[buffer.len - 1] == '\n') strbuf_setlen(&buffer, buffer.len - 1); + strbuf_reset(&exp); strbuf_addf(&exp, "# service=%s", service); if (strbuf_cmp(&exp, &buffer)) die("invalid server response; got '%s'", buffer.buf); @@ -160,6 +165,8 @@ static struct discovery* discover_refs(const char *service) } free(refs_url); + strbuf_release(&exp); + strbuf_release(&type); strbuf_release(&buffer); last_discovery = last; return last; diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh index 02f442bfad..895b9258b0 100644 --- a/t/lib-httpd.sh +++ b/t/lib-httpd.sh @@ -80,6 +80,7 @@ fi prepare_httpd() { mkdir -p "$HTTPD_DOCUMENT_ROOT_PATH" cp "$TEST_PATH"/passwd "$HTTPD_ROOT_PATH" + cp "$TEST_PATH"/broken-smart-http.sh "$HTTPD_ROOT_PATH" ln -s "$LIB_HTTPD_MODULE_PATH" "$HTTPD_ROOT_PATH/modules" diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf index fe76e84b74..938b4cf803 100644 --- a/t/lib-httpd/apache.conf +++ b/t/lib-httpd/apache.conf @@ -62,9 +62,13 @@ Alias /auth/dumb/ www/auth/dumb/ SetEnv GIT_COMMITTER_EMAIL custom@example.com ScriptAliasMatch /smart_*[^/]*/(.*) ${GIT_EXEC_PATH}/git-http-backend/$1 +ScriptAlias /broken_smart/ broken-smart-http.sh/ Options FollowSymlinks + + Options ExecCGI + Options ExecCGI diff --git a/t/lib-httpd/broken-smart-http.sh b/t/lib-httpd/broken-smart-http.sh new file mode 100755 index 0000000000..f7ebfffa80 --- /dev/null +++ b/t/lib-httpd/broken-smart-http.sh @@ -0,0 +1,11 @@ +#!/bin/sh +printf "Content-Type: text/%s\n" "html" +echo +printf "%s\n" "001e# service=git-upload-pack" +printf "%s" "0000" +printf "%s%c%s%s\n" \ + "00a58681d9f286a48b08f37b3a095330da16689e3693 HEAD" \ + 0 \ + " include-tag multi_ack_detailed multi_ack ofs-delta" \ + " side-band side-band-64k thin-pack no-progress shallow no-done " +printf "%s" "0000" diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh index c5cd2e348c..cb95b953b7 100755 --- a/t/t5551-http-fetch.sh +++ b/t/t5551-http-fetch.sh @@ -157,6 +157,12 @@ test_expect_success 'GIT_SMART_HTTP can disable smart http' ' test_must_fail git fetch) ' +test_expect_success 'invalid Content-Type rejected' ' + echo "fatal: invalid content-type text/html" >expect + test_must_fail git clone $HTTPD_URL/broken_smart/repo.git 2>actual + test_cmp expect actual +' + test -n "$GIT_TEST_LONG" && test_set_prereq EXPENSIVE test_expect_success EXPENSIVE 'create 50,000 tags in the repo' '