From 0227fa2a353ae7162c007f213cfb99c1709a7f15 Mon Sep 17 00:00:00 2001 From: Philip Kelley Date: Sat, 30 Mar 2013 21:36:04 -0400 Subject: [PATCH 1/4] Avoid pre-Win7 WinHTTP self-redirect quirk --- src/transports/winhttp.c | 137 +++++++++++++++++++++++++++++++++++---- 1 file changed, 126 insertions(+), 11 deletions(-) diff --git a/src/transports/winhttp.c b/src/transports/winhttp.c index d4d0179f8..ba5d1d5f1 100644 --- a/src/transports/winhttp.c +++ b/src/transports/winhttp.c @@ -58,6 +58,7 @@ typedef struct { const char *service_url; const wchar_t *verb; HINTERNET request; + wchar_t *request_uri; char *chunk_buffer; unsigned chunk_buffer_len; HANDLE post_body; @@ -145,10 +146,10 @@ static int winhttp_stream_connect(winhttp_stream *s) winhttp_subtransport *t = OWNING_SUBTRANSPORT(s); git_buf buf = GIT_BUF_INIT; char *proxy_url = NULL; - wchar_t url[GIT_WIN_PATH], ct[MAX_CONTENT_TYPE_LEN]; + wchar_t ct[MAX_CONTENT_TYPE_LEN]; wchar_t *types[] = { L"*/*", NULL }; BOOL peerdist = FALSE; - int error = -1; + int error = -1, wide_len; /* Prepare URL */ git_buf_printf(&buf, "%s%s", t->path, s->service_url); @@ -156,13 +157,31 @@ static int winhttp_stream_connect(winhttp_stream *s) if (git_buf_oom(&buf)) return -1; - git__utf8_to_16(url, GIT_WIN_PATH, git_buf_cstr(&buf)); + /* Convert URL to wide characters */ + wide_len = MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS, + git_buf_cstr(&buf), -1, NULL, 0); + + if (!wide_len) { + giterr_set(GITERR_OS, "Failed to measure string for wide conversion"); + goto on_error; + } + + s->request_uri = git__malloc(wide_len * sizeof(wchar_t)); + + if (!s->request_uri) + goto on_error; + + if (!MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS, + git_buf_cstr(&buf), -1, s->request_uri, wide_len)) { + giterr_set(GITERR_OS, "Failed to convert string to wide form"); + goto on_error; + } /* Establish request */ s->request = WinHttpOpenRequest( t->connection, s->verb, - url, + s->request_uri, NULL, WINHTTP_NO_REFERER, types, @@ -179,19 +198,36 @@ static int winhttp_stream_connect(winhttp_stream *s) if (proxy_url) { WINHTTP_PROXY_INFO proxy_info; - size_t wide_len; + wchar_t *proxy_wide; - git__utf8_to_16(url, GIT_WIN_PATH, proxy_url); + /* Convert URL to wide characters */ + wide_len = MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS, + proxy_url, -1, NULL, 0); - wide_len = wcslen(url); + if (!wide_len) { + giterr_set(GITERR_OS, "Failed to measure string for wide conversion"); + goto on_error; + } + + proxy_wide = git__malloc(wide_len * sizeof(wchar_t)); + + if (!proxy_wide) + goto on_error; + + if (!MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS, + proxy_url, -1, proxy_wide, wide_len)) { + giterr_set(GITERR_OS, "Failed to convert string to wide form"); + git__free(proxy_wide); + goto on_error; + } /* Strip any trailing forward slash on the proxy URL; * WinHTTP doesn't like it if one is present */ - if (L'/' == url[wide_len - 1]) - url[wide_len - 1] = L'\0'; + if (wide_len > 1 && L'/' == proxy_wide[wide_len - 2]) + proxy_wide[wide_len - 2] = L'\0'; proxy_info.dwAccessType = WINHTTP_ACCESS_TYPE_NAMED_PROXY; - proxy_info.lpszProxy = url; + proxy_info.lpszProxy = proxy_wide; proxy_info.lpszProxyBypass = NULL; if (!WinHttpSetOption(s->request, @@ -199,8 +235,11 @@ static int winhttp_stream_connect(winhttp_stream *s) &proxy_info, sizeof(WINHTTP_PROXY_INFO))) { giterr_set(GITERR_OS, "Failed to set proxy"); + git__free(proxy_wide); goto on_error; } + + git__free(proxy_wide); } /* Strip unwanted headers (X-P2P-PeerDist, X-P2P-PeerDistEx) that WinHTTP @@ -348,8 +387,15 @@ static int winhttp_stream_read( winhttp_stream *s = (winhttp_stream *)stream; winhttp_subtransport *t = OWNING_SUBTRANSPORT(s); DWORD dw_bytes_read; + char replay_count = 0; replay: + /* Enforce a reasonable cap on the number of replays */ + if (++replay_count >= 7) { + giterr_set(GITERR_NET, "Too many redirects or authentication replays"); + return -1; + } + /* Connect if necessary */ if (!s->request && winhttp_stream_connect(s) < 0) return -1; @@ -445,10 +491,74 @@ replay: WINHTTP_HEADER_NAME_BY_INDEX, &status_code, &status_code_length, WINHTTP_NO_HEADER_INDEX)) { - giterr_set(GITERR_OS, "Failed to retreive status code"); + giterr_set(GITERR_OS, "Failed to retrieve status code"); return -1; } + /* The implementation of WinHTTP prior to Windows 7 will not + * redirect to an identical URI. Some Git hosters use self-redirects + * as part of their DoS mitigation strategy. Check first to see if we + * have a redirect status code, and that we haven't already streamed + * a post body. (We can't replay a streamed POST.) */ + if (!s->chunked && + (HTTP_STATUS_MOVED == status_code || + HTTP_STATUS_REDIRECT == status_code || + (HTTP_STATUS_REDIRECT_METHOD == status_code && + get_verb == s->verb) || + HTTP_STATUS_REDIRECT_KEEP_VERB == status_code)) { + + /* Check for Windows 7. This workaround is only necessary on + * Windows Vista and earlier. Windows 7 is version 6.1. */ + DWORD dwVersion = GetVersion(); + + if (LOBYTE(LOWORD(dwVersion)) < 6 || + (LOBYTE(LOWORD(dwVersion)) == 6 && + HIBYTE(LOWORD(dwVersion)) < 1)) { + wchar_t *location; + DWORD location_length; + int redirect_cmp; + + /* OK, fetch the Location header from the redirect. */ + if (WinHttpQueryHeaders(s->request, + WINHTTP_QUERY_LOCATION, + WINHTTP_HEADER_NAME_BY_INDEX, + WINHTTP_NO_OUTPUT_BUFFER, + &location_length, + WINHTTP_NO_HEADER_INDEX) || + GetLastError() != ERROR_INSUFFICIENT_BUFFER) { + giterr_set(GITERR_OS, "Failed to read Location header"); + return -1; + } + + location = git__malloc(location_length); + GITERR_CHECK_ALLOC(location); + + if (!WinHttpQueryHeaders(s->request, + WINHTTP_QUERY_LOCATION, + WINHTTP_HEADER_NAME_BY_INDEX, + location, + &location_length, + WINHTTP_NO_HEADER_INDEX)) { + giterr_set(GITERR_OS, "Failed to read Location header"); + git__free(location); + return -1; + } + + /* Compare the Location header with the request URI */ + redirect_cmp = wcscmp(location, s->request_uri); + git__free(location); + + if (!redirect_cmp) { + /* Replay the request */ + WinHttpCloseHandle(s->request); + s->request = NULL; + s->sent_request = 0; + + goto replay; + } + } + } + /* Handle authentication failures */ if (HTTP_STATUS_DENIED == status_code && get_verb == s->verb && t->owner->cred_acquire_cb) { @@ -747,6 +857,11 @@ static void winhttp_stream_free(git_smart_subtransport_stream *stream) s->post_body = NULL; } + if (s->request_uri) { + git__free(s->request_uri); + s->request_uri = NULL; + } + if (s->request) { WinHttpCloseHandle(s->request); s->request = NULL; From 8cc2f2d86fa8a54b1ba87564c046e2e431e4ced0 Mon Sep 17 00:00:00 2001 From: Philip Kelley Date: Sun, 31 Mar 2013 12:10:27 -0400 Subject: [PATCH 2/4] Win32 error reporting: Support WinHTTP errors --- src/win32/error.c | 72 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 54 insertions(+), 18 deletions(-) diff --git a/src/win32/error.c b/src/win32/error.c index 3851ff099..4f169cf4d 100644 --- a/src/win32/error.c +++ b/src/win32/error.c @@ -8,34 +8,70 @@ #include "common.h" #include "error.h" +#ifdef GIT_WINHTTP +# include +#endif + +#define WC_ERR_INVALID_CHARS 0x80 + char *git_win32_get_error_message(DWORD error_code) { LPWSTR lpMsgBuf = NULL; + HMODULE hModule = NULL; + char *utf8_msg = NULL; + int utf8_size; + DWORD dwFlags = + FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_IGNORE_INSERTS; if (!error_code) return NULL; - if (FormatMessageW( - FORMAT_MESSAGE_ALLOCATE_BUFFER | - FORMAT_MESSAGE_FROM_SYSTEM | - FORMAT_MESSAGE_IGNORE_INSERTS, - NULL, error_code, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), - (LPWSTR)&lpMsgBuf, 0, NULL)) { - int utf8_size = WideCharToMultiByte(CP_UTF8, 0, lpMsgBuf, -1, NULL, 0, NULL, NULL); +#ifdef GIT_WINHTTP + /* Errors raised by WinHTTP are not in the system resource table */ + if (error_code >= WINHTTP_ERROR_BASE && + error_code <= WINHTTP_ERROR_LAST) + hModule = GetModuleHandleW(L"winhttp"); +#endif - char *lpMsgBuf_utf8 = git__malloc(utf8_size * sizeof(char)); - if (lpMsgBuf_utf8 == NULL) { - LocalFree(lpMsgBuf); - return NULL; - } - if (!WideCharToMultiByte(CP_UTF8, 0, lpMsgBuf, -1, lpMsgBuf_utf8, utf8_size, NULL, NULL)) { - LocalFree(lpMsgBuf); - git__free(lpMsgBuf_utf8); - return NULL; + GIT_UNUSED(hModule); + + if (hModule) + dwFlags |= FORMAT_MESSAGE_FROM_HMODULE; + else + dwFlags |= FORMAT_MESSAGE_FROM_SYSTEM; + + if (FormatMessageW(dwFlags, hModule, error_code, + MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), + (LPWSTR)&lpMsgBuf, 0, NULL)) { + + /* Invalid code point check supported on Vista+ only */ + if (LOBYTE(LOWORD(GetVersion())) >= 6) + dwFlags = WC_ERR_INVALID_CHARS; + else + dwFlags = 0; + + utf8_size = WideCharToMultiByte(CP_UTF8, dwFlags, + lpMsgBuf, -1, NULL, 0, NULL, NULL); + + if (!utf8_size) { + assert(0); + goto on_error; } + utf8_msg = git__malloc(utf8_size); + + if (!utf8_msg) + goto on_error; + + if (!WideCharToMultiByte(CP_UTF8, dwFlags, + lpMsgBuf, -1, utf8_msg, utf8_size, NULL, NULL)) { + git__free(utf8_msg); + goto on_error; + } + +on_error: LocalFree(lpMsgBuf); - return lpMsgBuf_utf8; } - return NULL; + + return utf8_msg; } From 5c5eeba6fda50e350c5e007ed5d59f8ca92ad0d8 Mon Sep 17 00:00:00 2001 From: Philip Kelley Date: Sun, 31 Mar 2013 22:22:33 -0400 Subject: [PATCH 3/4] Add git_has_win32_version helper --- src/common.h | 3 ++- src/transports/winhttp.c | 8 ++------ src/win32/error.c | 2 +- src/win32/version.h | 20 ++++++++++++++++++++ 4 files changed, 25 insertions(+), 8 deletions(-) create mode 100644 src/win32/version.h diff --git a/src/common.h b/src/common.h index 235da0412..02d9ce9b6 100644 --- a/src/common.h +++ b/src/common.h @@ -29,9 +29,10 @@ # include "win32/msvc-compat.h" # include "win32/mingw-compat.h" # include "win32/error.h" +# include "win32/version.h" # ifdef GIT_THREADS # include "win32/pthread.h" -#endif +# endif #else diff --git a/src/transports/winhttp.c b/src/transports/winhttp.c index ba5d1d5f1..e502001cb 100644 --- a/src/transports/winhttp.c +++ b/src/transports/winhttp.c @@ -509,11 +509,7 @@ replay: /* Check for Windows 7. This workaround is only necessary on * Windows Vista and earlier. Windows 7 is version 6.1. */ - DWORD dwVersion = GetVersion(); - - if (LOBYTE(LOWORD(dwVersion)) < 6 || - (LOBYTE(LOWORD(dwVersion)) == 6 && - HIBYTE(LOWORD(dwVersion)) < 1)) { + if (!git_has_win32_version(6, 1)) { wchar_t *location; DWORD location_length; int redirect_cmp; @@ -991,7 +987,7 @@ static int winhttp_receivepack( { /* WinHTTP only supports Transfer-Encoding: chunked * on Windows Vista (NT 6.0) and higher. */ - s->chunked = LOBYTE(LOWORD(GetVersion())) >= 6; + s->chunked = git_has_win32_version(6, 0); if (s->chunked) s->parent.write = winhttp_stream_write_chunked; diff --git a/src/win32/error.c b/src/win32/error.c index 4f169cf4d..4a9a0631f 100644 --- a/src/win32/error.c +++ b/src/win32/error.c @@ -45,7 +45,7 @@ char *git_win32_get_error_message(DWORD error_code) (LPWSTR)&lpMsgBuf, 0, NULL)) { /* Invalid code point check supported on Vista+ only */ - if (LOBYTE(LOWORD(GetVersion())) >= 6) + if (git_has_win32_version(6, 0)) dwFlags = WC_ERR_INVALID_CHARS; else dwFlags = 0; diff --git a/src/win32/version.h b/src/win32/version.h new file mode 100644 index 000000000..803cc60e2 --- /dev/null +++ b/src/win32/version.h @@ -0,0 +1,20 @@ +/* + * Copyright (C) the libgit2 contributors. All rights reserved. + * + * This file is part of libgit2, distributed under the GNU GPL v2 with + * a Linking Exception. For full terms see the included COPYING file. + */ +#ifndef INCLUDE_win32_version_h__ +#define INCLUDE_win32_version_h__ + +#include + +GIT_INLINE(int) git_has_win32_version(int major, int minor) +{ + WORD wVersion = LOWORD(GetVersion()); + + return LOBYTE(wVersion) > major || + (LOBYTE(wVersion) == major && HIBYTE(wVersion) >= minor); +} + +#endif \ No newline at end of file From b39f969732bbcdd30be5a43c72eb8b3e89696cd3 Mon Sep 17 00:00:00 2001 From: Philip Kelley Date: Sun, 31 Mar 2013 23:04:14 -0400 Subject: [PATCH 4/4] Fix whitespace in src/win32/version.h --- src/win32/version.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/win32/version.h b/src/win32/version.h index 803cc60e2..483962b57 100644 --- a/src/win32/version.h +++ b/src/win32/version.h @@ -9,7 +9,7 @@ #include -GIT_INLINE(int) git_has_win32_version(int major, int minor) +GIT_INLINE(int) git_has_win32_version(int major, int minor) { WORD wVersion = LOWORD(GetVersion()); @@ -17,4 +17,4 @@ GIT_INLINE(int) git_has_win32_version(int major, int minor) (LOBYTE(wVersion) == major && HIBYTE(wVersion) >= minor); } -#endif \ No newline at end of file +#endif