Re: Hans Leidekker : wininet: Make sure not to overwrite any caller supplied authorization header.

[Robert Shearman]

Alexandre Julliard wrote:
> Module: wine
> Branch: master
> Commit: b069ef4268e7056856ce6714714d929165f24fc8
> URL:    http://source.winehq.org/git/wine.git/?a=commit;h=b069ef4268e7056856ce671471...
>
> Author: Hans Leidekker <hans(a)it.vu.nl>
> Date:   Fri Feb  1 14:40:15 2008 +0100
>
> wininet: Make sure not to overwrite any caller supplied authorization header.
>
> ---
>
> dlls/wininet/http.c       |   36 ++++++++++--------------------------
>  dlls/wininet/tests/http.c |    5 ++++-
>  2 files changed, 14 insertions(+), 27 deletions(-)
>
> diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c
> index 65b86fb..a339c86 100644
> --- a/dlls/wininet/http.c
> +++ b/dlls/wininet/http.c
> @@ -1187,9 +1187,11 @@ static UINT HTTP_DecodeBase64( LPCWSTR base64, LPSTR bin )
>   *
>   *   Insert or delete the authorization field in the request header.
>   */
> -static BOOL HTTP_InsertAuthorizationForHeader( LPWININETHTTPREQW lpwhr, struct HttpAuthInfo *pAuthInfo, LPCWSTR header )
> +static BOOL HTTP_InsertAuthorization( LPWININETHTTPREQW lpwhr, LPCWSTR header, BOOL first )
>  {
>      WCHAR *authorization = NULL;
> +    struct HttpAuthInfo *pAuthInfo = lpwhr->pAuthInfo;
> +    DWORD flags;
>
> if (pAuthInfo && pAuthInfo->auth_data_len)
>      {
> @@ -1222,35 +1224,17 @@ static BOOL HTTP_InsertAuthorizationForHeader( LPWININETHTTPREQW lpwhr, struct H
>
> TRACE("Inserting authorization: %s\n", debugstr_w(authorization));
>
> -    HTTP_ProcessHeader(lpwhr, header, authorization,
> -                       HTTP_ADDHDR_FLAG_REPLACE | HTTP_ADDHDR_FLAG_REQ);
> +    /* make sure not to overwrite any caller supplied authorization header */
> +    flags = HTTP_ADDHDR_FLAG_REQ;
> +    flags |= first ? HTTP_ADDHDR_FLAG_ADD_IF_NEW : HTTP_ADDHDR_FLAG_REPLACE;
>
> -    HeapFree(GetProcessHeap(), 0, authorization);
> +    HTTP_ProcessHeader(lpwhr, header, authorization, flags);
>
> +    HeapFree(GetProcessHeap(), 0, authorization);
>      return TRUE;
>  }
>
> /***********************************************************************
> - *  HTTP_InsertAuthorization
> - *
> - *   Insert the authorization field in the request header
> - */
> -static BOOL HTTP_InsertAuthorization( LPWININETHTTPREQW lpwhr )
> -{
> -    return HTTP_InsertAuthorizationForHeader(lpwhr, lpwhr->pAuthInfo, szAuthorization);
> -}
> -
> -/***********************************************************************
> - *  HTTP_InsertProxyAuthorization
> - *
> - *   Insert the proxy authorization field in the request header
> - */
> -static BOOL HTTP_InsertProxyAuthorization( LPWININETHTTPREQW lpwhr )
> -{
> -    return HTTP_InsertAuthorizationForHeader(lpwhr, lpwhr->pProxyAuthInfo, szProxy_Authorization);
> -}
> -
> -/***********************************************************************
>   *           HTTP_DealWithProxy
>   */
>  static BOOL HTTP_DealWithProxy( LPWININETAPPINFOW hIC,
> @@ -2621,8 +2605,8 @@ BOOL WINAPI HTTP_HttpSendRequestW(LPWININETHTTPREQW lpwhr, LPCWSTR lpszHeaders,
>                             lpwhr->hdr.dwFlags & INTERNET_FLAG_KEEP_CONNECTION ? szKeepAlive : szClose,
>                             HTTP_ADDHDR_FLAG_REQ | HTTP_ADDHDR_FLAG_REPLACE);
>
> -        HTTP_InsertAuthorization(lpwhr);
> -        HTTP_InsertProxyAuthorization(lpwhr);
> +        HTTP_InsertAuthorization(lpwhr, szAuthorization, !loop_next);
> +        HTTP_InsertAuthorization(lpwhr, szProxy_Authorization, !loop_next);
>
> /* add the headers the caller supplied */
>          if( lpszHeaders && dwHeaderLength )
> diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c
> index 8bb7cca..88369cf 100644
> --- a/dlls/wininet/tests/http.c
> +++ b/dlls/wininet/tests/http.c
> @@ -1502,7 +1502,10 @@ static void test_header_handling_order(int port)
>      request = HttpOpenRequest(connect, NULL, "/test3", NULL, NULL, types, INTERNET_FLAG_KEEP_CONNECTION, 0);
>      ok(request != NULL, "HttpOpenRequest failed\n");
>
> -    ret = HttpSendRequest(request, authorization, ~0UL, NULL, 0);
> +    ret = HttpAddRequestHeaders(request, authorization, ~0UL, HTTP_ADDREQ_FLAG_ADD);
> +    ok(ret, "HttpAddRequestHeaders failed\n");
> +
> +    ret = HttpSendRequest(request, NULL, 0, NULL, 0);
>      ok(ret, "HttpSendRequest failed\n");
>
> status = 0;

This patch simultaneously breaks proxy authentication and NTLM 
authentication. Proxy authentication is broken because the helper 
function is removed that passes lpwhr->pProxyAuthInfo into 
HTTP_InsertAuthentication. NTLM authentication is broken because first 
is false when we try to send the authenticate message (i.e. the 3rd leg) 
and so the data for the negotiate message (i.e. the first leg) is sent 
instead as the field is not overridden.

Is this trying to fix a real app? If not, why isn't the code that is 
using the behaviour using INTERNET_FLAG_NO_AUTH?

-- 
Rob Shearman