Re: [Bug-wget] Issue in cookie path checking

[Darshit Shah]

Hi Yasuhisa,

Thanks for the patch. The cookie domain patch checking code in Wget
was old and only based on a heuristic. However, we are currently in
the process of using libpsl a library that handles this for us. I have
submitted a patch which is currently in the pipeline that adds support
for using libpsl to perform cookie domain name checking.

Your code may stil be useful in the fallback mechanism. Could you
please send a patch file as generated by `git format-patch` against
the current HEAD of the tree and also add the details to the relevant
ChangeLog file? It would make applying the patch so much easier for
us.

On Thu, May 8, 2014 at 8:12 PM, address@hidden
<address@hidden> wrote:
> Hi all,
>
>  I found two issues in path checking code in cookie.c.
>
>  In cookie_handle_set_cookie(), path in Set-Cookie header should be
> checked so as not to be accepted when it is upper than that of
> requested document.
>
>  However, current implementation works as:
>
> - check_path_match() validate the path of requested document
>   when its prefix is same with cookie_path.
>   path_matches(full_path, prefix) checks if full_path starts with prefix.
>   Current code allows /foo/bar/test.html to issue path=/ cookie.
>   Expected behavior is opposite. cookie_path must be child of current path.
>
> - cookie->path is compared with path(full document path including filename)
>   in stead of its parent path.
>
>  I applied following fix, and it works as expected. Please consider to merge 
> this fix in next release.
>
> $ diff -c wget-1.15/src/cookies.c.orig wget-1.15/src/cookies.c
> *** wget-1.15/src/cookies.c.orig        2013-10-21 23:50:12.000000000 +0900
> --- wget-1.15/src/cookies.c     2014-05-08 22:47:57.317467164 +0900
> ***************
> *** 634,640 ****
>   static bool
>   check_path_match (const char *cookie_path, const char *path)
>   {
> !   return path_matches (path, cookie_path) != 0;
>   }
>
>   /* Prepend '/' to string S.  S is copied to fresh stack-allocated
> --- 634,640 ----
>   static bool
>   check_path_match (const char *cookie_path, const char *path)
>   {
> !   return path_matches (cookie_path, path) != 0;
>   }
>
>   /* Prepend '/' to string S.  S is copied to fresh stack-allocated
> ***************
> *** 707,713 ****
>     else
>       {
>         /* The cookie sets its own path; verify that it is legal. */
> !       if (!check_path_match (cookie->path, path))
>           {
>             DEBUGP (("Attempt to fake the path: %s, %s\n",
>                      cookie->path, path));
> --- 707,714 ----
>     else
>       {
>         /* The cookie sets its own path; verify that it is legal. */
> !       char *trailing_slash = strrchr (path, '/');
> !       if (!check_path_match (cookie->path, trailing_slash ? strdupdelim 
> (path, trailing_slash + 1) : '/'))
>           {
>             DEBUGP (("Attempt to fake the path: %s, %s\n",
>                      cookie->path, path));
> $
>
> Thanks,
> Yasuhisa Ishikawa



-- 
Thanking You,
Darshit Shah