[PATCH v2 0/1] MR9339: winhttp/tests: Avoid buffer-overflow in WinHttpCreateUrl_test (ASan).
The test sets in fill_url_components the dwPasswordLength to 8, therefore the memcpy copies 8 wide characters. But before this patch variable empty allocates just one wide character. [ASan report details](https://gitlab.winehq.org/bernhardu/wine/-/blob/asan-pe_2025-10-18_wine-10.1...) -- v2: winhttp/tests: Fix setting string lengths in WinHttpCreateUrl_test (ASan). https://gitlab.winehq.org/wine/wine/-/merge_requests/9339
From: Bernhard Übelacker <bernhardu(a)mailbox.org> --- dlls/winhttp/tests/url.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/dlls/winhttp/tests/url.c b/dlls/winhttp/tests/url.c index fadf8b850ca..c06ff7548ce 100644 --- a/dlls/winhttp/tests/url.c +++ b/dlls/winhttp/tests/url.c @@ -40,7 +40,7 @@ static WCHAR escape3[] = {'?','t','e','x','t','=',0xfb00,0}; static WCHAR escape4[] = {'/','t','e','x','t','=',0xfb00,0}; static const WCHAR url1[] = L"http://username:password(a)www.winehq.org/site/about?query"; -static const WCHAR url2[] = L"http://username:"; +static const WCHAR url2[] = L"http://username:@www.winehq.org/site/about?query"; static const WCHAR url3[] = L"http://www.winehq.org/site/about?query"; static const WCHAR url4[] = L"http://"; static const WCHAR url5[] = L"ftp://username:password(a)www.winehq.org:80/site/about?query"; @@ -62,6 +62,7 @@ static const WCHAR url19[] = L"http://?text=\xfb00"; static const WCHAR url20[] = L"http:///text=\xfb00"; static const WCHAR url21[] = L"https://nba2k19-ws.2ksports.com:19133/nba/v4/Accounts/get_account?x=37895267..."; static const WCHAR url22[] = L"http://winehq.org:/"; +static const WCHAR url23[] = L"http://:@www.winehq.org/site/about?query"; static const WCHAR url_k1[] = L"http://username:password(a)www.winehq.org/site/about"; static const WCHAR url_k2[] = L"http://www.winehq.org"; @@ -202,11 +203,12 @@ static void WinHttpCreateUrl_test( void ) /* valid username, empty password */ fill_url_components( &uc ); uc.lpszPassword = empty; + uc.dwPasswordLength = 0; url[0] = 0; len = 256; ret = WinHttpCreateUrl( &uc, 0, url, &len ); ok( ret, "expected success\n" ); - ok( len == 56, "expected len 56 got %lu\n", len ); + ok( len == 48, "expected len 48 got %lu\n", len ); ok( !lstrcmpW( url, url2 ), "url doesn't match\n" ); /* valid password, NULL username */ @@ -222,6 +224,7 @@ static void WinHttpCreateUrl_test( void ) /* valid password, empty username */ fill_url_components( &uc ); uc.lpszUserName = empty; + uc.dwUserNameLength = 0; url[0] = 0; len = 256; ret = WinHttpCreateUrl( &uc, 0, url, &len ); @@ -241,13 +244,15 @@ static void WinHttpCreateUrl_test( void ) /* empty username, empty password */ fill_url_components( &uc ); uc.lpszUserName = empty; + uc.dwUserNameLength = 0; uc.lpszPassword = empty; + uc.dwPasswordLength = 0; url[0] = 0; len = 256; ret = WinHttpCreateUrl( &uc, 0, url, &len ); ok( ret, "expected success\n" ); - ok( len == 56, "expected len 56 got %lu\n", len ); - ok( !lstrcmpW( url, url4 ), "url doesn't match\n" ); + ok( len == 40, "expected len 40 got %lu\n", len ); + ok( !lstrcmpW( url, url23 ), "url doesn't match\n" ); /* nScheme has lower precedence than lpszScheme */ fill_url_components( &uc ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9339
v2: - Instead of allocating 8 character for `empty`, setting dwPasswordLength and dwUserNameLength to zero in the input struct. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9339#note_120498
On Mon Nov 3 13:01:12 2025 +0000, Hans Leidekker wrote:
Please set dwPasswordLength to zero in that test instead. Thanks for looking into it, I changed it in v2, but it needed a few more changes to the test.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9339#note_120499
This merge request was approved by Hans Leidekker. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9339
participants (2)
-
Bernhard Übelacker -
Hans Leidekker (＠hans)