[PATCH 1/3] wininet/tests: Test InternetCrackUrl when given a bogus length

[Tim Clem]

Counter-Strike: Source was doing exactly that when trying to parse URLs for custom maps.
They seem to pass the length of the whole buffer they allocated, all the time (512).
InternetCrackURL tries to parse the entirety of the buffer, so it it will try to copy
whatever it finds into the appropriate places. In CS:S's case, the buffer looked like
this: "http://example.com/map.bsp\0\0(other non-printable garbage)", and CrackURL tried
to copy "map.bsp\0\0..." into lpszUrlPath.

I assume if the things after the null looked like valid URL components, it would
silently put a string with a null into one of the URL_COMPONENTS fields. For instance,
I bet (but didn't test) that "http://x\0.org:8080/path" would parse, and just leave
"x\0.org" in lpszHostName.

Notably, if you look at the ICU_DECODE path in InternetCrackURLW, it first duplicates
the URL with heap_strndupW, which will stop at the first null, so that case currently
has different (correct) behavior.

July 2, 2021 9:11 AM, "Jacek Caban" <jacek at codeweavers.com> wrote:

> Hi Tim,
> 
> On 7/1/21 7:40 PM, Tim Clem wrote:
> 
>> + /* Windows treats dwUrlLength as a maximum - if there is a null before
>> + * that length, it stops there. */
>> + copy_compsA(&urlSrc, &urlComponents, 32, 1024, 1024, 1024, 1024, 1024);
>> + ret = InternetCrackUrlA("http://x.org", 13 /* includes the nul */, 0, &urlComponents);
>> + ok(ret, "InternetCrackUrlA failed with error %d\n", GetLastError());
>> + todo_wine
>> + ok(urlComponents.dwHostNameLength == 5,
>> + "Expected dwHostNameLength of 5, got %d\n", urlComponents.dwHostNameLength);
> 
> That test is fine on its own, but for the fix it would be also interesting to know what happens if
> you pass even larger value (that is, what if the length includes some characters after trailing
> null byte)?
> 
> Thanks,
> 
> Jacek