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

[yasuhisa . ishikawa]

Hi Darshit,

 Sorry to be late. I have never used git so it takes long for me to learn
how to generate path using git.

 I’m not sure I have done it well. If there are something wrong with this
attachment, please correct.

0001-2014-06-12-Yasuhisa-Ishikawa-yasuhisa.ishikawa-kuman.patch
Description: Binary data

Regards,
Yasuhisa Ishikawa

2014/06/04 3:09、Darshit Shah <address@hidden> のメール：

> 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