[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-wget] Issue in cookie path checking
From: |
yasuhisa . ishikawa |
Subject: |
Re: [Bug-wget] Issue in cookie path checking |
Date: |
Thu, 12 Jun 2014 23:39:48 +0900 |
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