[PATCH v2 0/2] MR8449: Draft: wininet: Improve invalid parameter handling.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58418 -- v2: wininet: Improve invalid parameter handling in HTTPREQ_QueryOption. wininet: Improve invalid parameter handling in APPINFO_QueryOption. https://gitlab.winehq.org/wine/wine/-/merge_requests/8449
From: Bernhard Übelacker <bernhardu(a)mailbox.org> --- dlls/wininet/internet.c | 2 ++ dlls/wininet/tests/http.c | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/dlls/wininet/internet.c b/dlls/wininet/internet.c index 17a7ce9385e..545718cdafe 100644 --- a/dlls/wininet/internet.c +++ b/dlls/wininet/internet.c @@ -1055,6 +1055,8 @@ static DWORD APPINFO_QueryOption(object_header_t *hdr, DWORD option, void *buffe TRACE("INTERNET_OPTION_USER_AGENT\n"); + if(!size) return ERROR_INVALID_PARAMETER; + bufsize = *size; if (unicode) { diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c index d63f42e27d7..b368f2a7dcb 100644 --- a/dlls/wininet/tests/http.c +++ b/dlls/wininet/tests/http.c @@ -3149,6 +3149,25 @@ static void test_proxy_direct(int port) r = InternetSetOptionA(hi, INTERNET_OPTION_USER_AGENT, useragent, 1); ok(r, "failed to set useragent\n"); + SetLastError(0xdeadbeef); + r = InternetQueryOptionA(hi, INTERNET_OPTION_USER_AGENT, NULL, NULL); + ok(GetLastError() == ERROR_INVALID_PARAMETER, "got %lu\n", GetLastError()); + ok(!r, "unexpected success\n"); + + sz = 0; + SetLastError(0xdeadbeef); + r = InternetQueryOptionA(hi, INTERNET_OPTION_USER_AGENT, NULL, &sz); + ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "got %lu\n", GetLastError()); + ok(!r, "unexpected success\n"); + ok(sz == strlen(useragent) + 1, "got %lu\n", sz); + + sz = 0xbeef; + SetLastError(0xdeadbeef); + r = InternetQueryOptionA(hi, INTERNET_OPTION_USER_AGENT, NULL, &sz); + ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "got %lu\n", GetLastError()); + ok(!r, "unexpected success\n"); + ok(sz == strlen(useragent) + 1, "got %lu\n", sz); + buffer[0] = 0; sz = 0; SetLastError(0xdeadbeef); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8449
From: Bernhard Übelacker <bernhardu(a)mailbox.org> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58418 --- dlls/wininet/http.c | 8 ++++-- dlls/wininet/tests/http.c | 60 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c index 497a582ff89..dc0ce09141a 100644 --- a/dlls/wininet/http.c +++ b/dlls/wininet/http.c @@ -1966,13 +1966,17 @@ static void HTTPREQ_CloseConnection(object_header_t *hdr) static DWORD str_to_buffer(const WCHAR *str, void *buffer, DWORD *size, BOOL unicode) { int len; + + if (!size) + return ERROR_INVALID_PARAMETER; + if (unicode) { WCHAR *buf = buffer; if (str) len = lstrlenW(str); else len = 0; - if (*size < (len + 1) * sizeof(WCHAR)) + if (*size < (len + 1) * sizeof(WCHAR) || !buf) { *size = (len + 1) * sizeof(WCHAR); return ERROR_INSUFFICIENT_BUFFER; @@ -1989,7 +1993,7 @@ static DWORD str_to_buffer(const WCHAR *str, void *buffer, DWORD *size, BOOL uni if (str) len = WideCharToMultiByte(CP_ACP, 0, str, -1, NULL, 0, NULL, NULL); else len = 1; - if (*size < len) + if (*size < len || !buf) { *size = len; return ERROR_INSUFFICIENT_BUFFER; diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c index b368f2a7dcb..394d3e5baf6 100644 --- a/dlls/wininet/tests/http.c +++ b/dlls/wininet/tests/http.c @@ -3013,6 +3013,11 @@ static void test_proxy_direct(int port) hr = HttpOpenRequestA(hc, NULL, "/test2", NULL, NULL, NULL, 0, 0); ok(hr != NULL, "HttpOpenRequest failed\n"); + SetLastError(0xdeadbeef); + r = InternetQueryOptionA(hr, INTERNET_OPTION_PROXY_PASSWORD, NULL, NULL); + ok(GetLastError() == ERROR_INVALID_PARAMETER, "got %lu\n", GetLastError()); + ok(!r, "unexpected success\n"); + sz = 0; SetLastError(0xdeadbeef); r = InternetQueryOptionA(hr, INTERNET_OPTION_PROXY_PASSWORD, NULL, &sz); @@ -3020,6 +3025,18 @@ static void test_proxy_direct(int port) ok(!r, "unexpected success\n"); ok(sz == 1, "got %lu\n", sz); + sz = 0xbeef; + SetLastError(0xdeadbeef); + r = InternetQueryOptionA(hr, INTERNET_OPTION_PROXY_PASSWORD, NULL, &sz); + ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "got %lu\n", GetLastError()); + ok(!r, "unexpected success\n"); + ok(sz == 1, "got %lu\n", sz); + + SetLastError(0xdeadbeef); + r = InternetQueryOptionA(hr, INTERNET_OPTION_PROXY_USERNAME, NULL, NULL); + ok(GetLastError() == ERROR_INVALID_PARAMETER, "got %lu\n", GetLastError()); + ok(!r, "unexpected success\n"); + sz = 0; SetLastError(0xdeadbeef); r = InternetQueryOptionA(hr, INTERNET_OPTION_PROXY_USERNAME, NULL, &sz); @@ -3027,6 +3044,13 @@ static void test_proxy_direct(int port) ok(!r, "unexpected success\n"); ok(sz == 1, "got %lu\n", sz); + sz = 0xbeef; + SetLastError(0xdeadbeef); + r = InternetQueryOptionA(hr, INTERNET_OPTION_PROXY_USERNAME, NULL, &sz); + ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "got %lu\n", GetLastError()); + ok(!r, "unexpected success\n"); + ok(sz == 1, "got %lu\n", sz); + sz = sizeof(buffer); SetLastError(0xdeadbeef); r = InternetQueryOptionA(hr, INTERNET_OPTION_PROXY_PASSWORD, buffer, &sz); @@ -3039,6 +3063,11 @@ static void test_proxy_direct(int port) ok(r, "unexpected failure %lu\n", GetLastError()); ok(!sz, "got %lu\n", sz); + SetLastError(0xdeadbeef); + r = InternetQueryOptionA(hr, INTERNET_OPTION_PASSWORD, NULL, NULL); + ok(GetLastError() == ERROR_INVALID_PARAMETER, "got %lu\n", GetLastError()); + ok(!r, "unexpected success\n"); + sz = 0; SetLastError(0xdeadbeef); r = InternetQueryOptionA(hr, INTERNET_OPTION_PASSWORD, NULL, &sz); @@ -3046,6 +3075,18 @@ static void test_proxy_direct(int port) ok(!r, "unexpected success\n"); ok(sz == 1, "got %lu\n", sz); + sz = 0xbeef; + SetLastError(0xdeadbeef); + r = InternetQueryOptionA(hr, INTERNET_OPTION_PASSWORD, NULL, &sz); + ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "got %lu\n", GetLastError()); + ok(!r, "unexpected success\n"); + ok(sz == 1, "got %lu\n", sz); + + SetLastError(0xdeadbeef); + r = InternetQueryOptionA(hr, INTERNET_OPTION_USERNAME, NULL, NULL); + ok(GetLastError() == ERROR_INVALID_PARAMETER, "got %lu\n", GetLastError()); + ok(!r, "unexpected success\n"); + sz = 0; SetLastError(0xdeadbeef); r = InternetQueryOptionA(hr, INTERNET_OPTION_USERNAME, NULL, &sz); @@ -3053,6 +3094,13 @@ static void test_proxy_direct(int port) ok(!r, "unexpected success\n"); ok(sz == 1, "got %lu\n", sz); + sz = 0xbeef; + SetLastError(0xdeadbeef); + r = InternetQueryOptionA(hr, INTERNET_OPTION_USERNAME, NULL, &sz); + ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "got %lu\n", GetLastError()); + ok(!r, "unexpected success\n"); + ok(sz == 1, "got %lu\n", sz); + sz = sizeof(buffer); SetLastError(0xdeadbeef); r = InternetQueryOptionA(hr, INTERNET_OPTION_PASSWORD, buffer, &sz); @@ -3065,6 +3113,11 @@ static void test_proxy_direct(int port) ok(r, "unexpected failure %lu\n", GetLastError()); ok(!sz, "got %lu\n", sz); + SetLastError(0xdeadbeef); + r = InternetQueryOptionA(hr, INTERNET_OPTION_URL, NULL, NULL); + ok(GetLastError() == ERROR_INVALID_PARAMETER, "got %lu\n", GetLastError()); + ok(!r, "unexpected success\n"); + sz = 0; SetLastError(0xdeadbeef); r = InternetQueryOptionA(hr, INTERNET_OPTION_URL, NULL, &sz); @@ -3072,6 +3125,13 @@ static void test_proxy_direct(int port) ok(!r, "unexpected success\n"); ok(sz == 34, "got %lu\n", sz); + sz = 0xbeef; + SetLastError(0xdeadbeef); + r = InternetQueryOptionA(hr, INTERNET_OPTION_URL, NULL, &sz); + ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "got %lu\n", GetLastError()); + ok(!r, "unexpected success\n"); + ok(sz == 34, "got %lu\n", sz); + sz = sizeof(buffer); SetLastError(0xdeadbeef); r = InternetQueryOptionA(hr, INTERNET_OPTION_URL, buffer, &sz); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8449
changes in v2: - return on size being NULL ERROR_INVALID_PARAMETER instead of ERROR_INSUFFICIENT_BUFFER. [Testbot run with first patch](https://testbot.winehq.org/JobDetails.pl?Key=158998) [Testbot run with both patches](https://testbot.winehq.org/JobDetails.pl?Key=159000) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8449#note_108206
rebase again and move the !buf check in the first patch to before the size check -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8449#note_122305
On Sun Nov 16 10:42:03 2025 +0000, Vijay Kiran Kamuju wrote:
rebase again and move the !buf check in the first patch to before the size check Thanks for having a looks.
Sorry to ask but you want the `!buf` to come first in the if conditions like this? ``` + if (!buf || *size < (len + 1) * sizeof(WCHAR)) + if (!buf || *size < len) ``` Or you like the `!buf` before the `if (!size)` in line 1970? But then we are not going to set the `*size` value? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8449#note_122467
participants (2)
-
Bernhard Übelacker -
Vijay Kiran Kamuju (＠infyquest)