Re: urlmon: Fix incorrect pointer arithmetic (too conversative) in map_url_to_zone. (RESEND)

[Ken Thomases]

On Jun 4, 2011, at 8:02 AM, Gerald Pfeifer wrote:
> Resending:  This really looks like a straightforward bug fix and the
> current code definitely wrong???

No.  As others have pointed out, your logic is wrong.  The existing code is correct.
> The difference between two pointers (of the same type) is the number
> of elements, not the number of bytes.  Thus the code below was way 
> incorrect, luckily only too conversative.

So, ptr-path is the number of elements between the two pointers.  But sizeof(root) is a number of bytes.  The precise reason to divide the latter by sizeof(WCHAR) is to arrive at a number of elements so it is proper to compare to ptr-path.

Put another way, look a bit lower in the code:
>         memcpy(root, path, (ptr-path)*sizeof(WCHAR));

It is clear that (ptr-path)*sizeof(WCHAR), a measure of bytes, must be no larger than the size of root in bytes.  Thus, this is the requirement:

(ptr-path)*sizeof(WCHAR) <= sizeof(root)

Dividing both sides by sizeof(WCHAR) gives an equivalent requirement:

(ptr-path) <= sizeof(root)/sizeof(WCHAR)

which is exactly what the code, as is, tests.  (Except that the current code doesn't allow for the equal case, in order to preserve a null terminator.)

Regards,
Ken
> dlls/urlmon/sec_mgr.c |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/dlls/urlmon/sec_mgr.c b/dlls/urlmon/sec_mgr.c
> index 7b4bb35..75850ee 100644
> --- a/dlls/urlmon/sec_mgr.c
> +++ b/dlls/urlmon/sec_mgr.c
> @@ -529,7 +529,7 @@ static HRESULT map_url_to_zone(LPCWSTR url, DWORD *zone, LPWSTR *ret_url)
>         hres = CoInternetParseUrl(secur_url, PARSE_PATH_FROM_URL, 0, path,
>                 sizeof(path)/sizeof(WCHAR), &size, 0);
> -
>    if(SUCCEEDED(hres) && (ptr = strchrW(path, '\\')) && ptr-path < sizeof(root)/sizeof(WCHAR)) {
> -
>    if(SUCCEEDED(hres) && (ptr = strchrW(path, '\\')) && ptr-path < sizeof(root)) {
>       UINT type;
>
>       memcpy(root, path, (ptr-path)*sizeof(WCHAR));
>
> -- 
> 1.7.4.1