[PATCH 0/1] MR8228: mshtml: Fix misuse of IWinInetHttpInfo_QueryInfo.

[Yuxuan Shui (＠yshui)]

IWinInetHttpInfo_QueryInfo returns a multibyte string, not a wide string. We
were also wrongly expecting it to have a NUL terminator.

* * *

this one doesn't feel good. IWinInetHttpInfo_QueryInfo calls HttpInfo_QueryInfo (urlmon), which calls HttpQueryInfoA (wininet), so it returns char*. but inside, HttpQueryInfoA is calling HttpQueryInfoW and does charset conversion. so we are converting the string to multibyte and back.

maybe there's an API that returns wchar* i don't know about.

[PATCH 1/1] mshtml: Fix misuse of IWinInetHttpInfo_QueryInfo.

[Yuxuan Shui]

From: Yuxuan Shui

IWinInetHttpInfo_QueryInfo returns a multibyte string, not a wide string. We
were also wrongly expecting it to have a NUL terminator.
---
 dlls/mshtml/navigate.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/dlls/mshtml/navigate.c b/dlls/mshtml/navigate.c
index 6b5eff15105..91e7d673227 100644
--- a/dlls/mshtml/navigate.c
+++ b/dlls/mshtml/navigate.c
@@ -813,8 +813,9 @@ static HRESULT process_response_headers(nsChannelBSC *This, const WCHAR *headers
 static void query_http_info(nsChannelBSC *This, IWinInetHttpInfo *wininet_info)
 {
     const WCHAR *ptr;
-    DWORD len = 0;
-    WCHAR *buf;
+    DWORD len = 0, wlen;
+    WCHAR *wbuf;
+    char *buf;

IWinInetHttpInfo_QueryInfo(wininet_info, HTTP_QUERY_RAW_HEADERS_CRLF, NULL, &len, NULL, NULL);
     if(!len)
@@ -825,17 +826,29 @@ static void query_http_info(nsChannelBSC *This, IWinInetHttpInfo *wininet_info)
         return;

IWinInetHttpInfo_QueryInfo(wininet_info, HTTP_QUERY_RAW_HEADERS_CRLF, buf, &len, NULL, NULL);
-    if(!len) {
-        free(buf);
-        return;
-    }
+    if(!len)
+        goto out;
+
+
+    wlen = MultiByteToWideChar(CP_ACP, 0, buf, len, NULL, 0) * sizeof(WCHAR);
+    if (!wlen)
+        goto out;
+
+    wbuf = malloc(wlen + sizeof(WCHAR));
+    if (!wbuf)
+        goto out;
+
+    MultiByteToWideChar(CP_ACP, 0, buf, len, wbuf, wlen / sizeof(WCHAR));
+    wbuf[wlen] = L'\0';

-    ptr = wcschr(buf, '\r');
+    ptr = wcschr(wbuf, '\r');
     if(ptr && ptr[1] == '\n') {
         ptr += 2;
         process_response_headers(This, ptr);
     }
+    free(wbuf);

+out:
     free(buf);
 }

[Gabriel Ivăncescu (＠insn)]

Gabriel Ivăncescu (@insn) commented about dlls/mshtml/navigate.c:
> -
>    return;
> - }
> - if(!len)
> -
>    goto out;
> -
> -
> - wlen = MultiByteToWideChar(CP_ACP, 0, buf, len, NULL, 0) * sizeof(WCHAR);
> - if (!wlen)
> -
>    goto out;
> -
> - wbuf = malloc(wlen + sizeof(WCHAR));
> - if (!wbuf)
> -
>    goto out;
> -
> - MultiByteToWideChar(CP_ACP, 0, buf, len, wbuf, wlen / sizeof(WCHAR));
> - wbuf[wlen] = L'\0';

According to the documentation, this uses the `ISO-8859-1` codepage, so maybe you should pass `28591`, but I'm not completely sure if that's true because I'm not super familiar with wininet (where this boils down to), but we don't seem to have any non-ASCII tests for headers in there?

Anyway, setting `wlen` to the size seems a bit counter-intuitive and you messed up because of it. wbuf[wlen] is wrong and out of bounds since it implicitly multiplies by sizeof(WCHAR). You can just store the length in there and then you won't even have to do the division. This is simpler IMO:

```c
    if(!(wlen = MultiByteToWideChar(28591, 0, buf, len, NULL, 0)))
        goto out;

if(!(wbuf = malloc((wlen + 1) * sizeof(WCHAR))))
        goto out;

MultiByteToWideChar(28591, 0, buf, len, wbuf, wlen);
    wbuf[wlen] = L'\0';
```

Sadly I can't review the codepage part properly since it's really wininet, we'll need to wait for Jacek for it.

[Yuxuan Shui (＠yshui)]

> you messed up because of it.

~~`wbuf` is allocated to be `wlen + sizeof(WCHAR)`. if it were out-of-bounds ASan would've caught it :)~~

oh sorry i am dumb

[Yuxuan Shui (＠yshui)]

On Fri Jun  6 17:16:40 2025 +0000, Yuxuan Shui wrote:
> > you messed up because of it.
>
> ~~`wbuf` is allocated to be `wlen + sizeof(WCHAR)`. if it were
> out-of-bounds ASan would've caught it :)~~
> oh sorry i am dumb

this was so out-of-bound it went past the ASan redzone :disappointed: