New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 632606 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Crash in net::cookie_util::GetCookieDomainWithString

Project Member Reported by ClusterFuzz, Jul 29 2016

Issue description

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5648955061043200

Fuzzer: libfuzzer_net_url_request_fuzzer
Job Type: libfuzzer_chrome_asan
Platform Id: linux

Crash Type: UNKNOWN
Crash Address: 0x03e90000067e
Crash State:
  net::cookie_util::GetCookieDomainWithString
  net::GetCookieDomain
  net::CanonicalCookie::Create
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan&range=408389:408457

Minimized Testcase (0.08 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv96dfdXviaLxX3NGlC5WV-1WNXFtuVE15cG22CcT2ehY7IRvb63r1e7InS7FZN0ew6yyFhtq-6laprSB8O1CpGdnrPkzpMRL1lSaB2h-OCHUfW6SP5klBDMwn16glOy59s8kEGJeL9lGH6JkwrUi75HV2TsZZw?testcase_id=5648955061043200
HTTP 308
Location://./(HFDP:0 

ODnvol:(no-sthTTP
Set-Cookie:�foo=barttp://foo/o`e


Filer: rnimmagadda

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.
 
Components: Tools>Test>FindIt>CorrectResult
Labels: -Pri-1 -Type-Bug M-54 Findit-for-crash Te-Logged Pri-2 Type-Bug-Regression
Owner: droger@chromium.org
Status: Assigned (was: Untriaged)
Suspecting:

Author: droger@google.com
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src//+/2fb376add13b6f658741e804bc226ac87b68698b
Time: Thu Nov 17 09:22:03 2011
The CL last changed line 50 of file cookie_util.cc, which is stack frame 3.

@droger: Could you please look into this issue.

Thank you.
Project Member

Comment 2 by ClusterFuzz, Aug 1 2016

ClusterFuzz has detected this issue as fixed in range 408588:408608.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5648955061043200

Fuzzer: libfuzzer_net_url_request_fuzzer
Job Type: libfuzzer_chrome_asan
Platform Id: linux

Crash Type: UNKNOWN
Crash Address: 0x03e90000067e
Crash State:
  net::cookie_util::GetCookieDomainWithString
  net::GetCookieDomain
  net::CanonicalCookie::Create
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan&range=408389:408457
Fixed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan&range=408588:408608

Minimized Testcase (0.08 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv96dfdXviaLxX3NGlC5WV-1WNXFtuVE15cG22CcT2ehY7IRvb63r1e7InS7FZN0ew6yyFhtq-6laprSB8O1CpGdnrPkzpMRL1lSaB2h-OCHUfW6SP5klBDMwn16glOy59s8kEGJeL9lGH6JkwrUi75HV2TsZZw?testcase_id=5648955061043200
HTTP 308
Location://./(HFDP:0 

ODnvol:(no-sthTTP
Set-Cookie:�foo=barttp://foo/o`e


See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 3 by ClusterFuzz, Aug 1 2016

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase is verified as fixed, closing issue.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Labels: ClusterFuzz-Wrong
Status: Assigned (was: Verified)
Project Member

Comment 5 by ClusterFuzz, Aug 2 2016

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5502965303738368

Fuzzer: libfuzzer_net_url_request_fuzzer
Job Type: libfuzzer_chrome_asan
Platform Id: linux

Crash Type: UNKNOWN
Crash Address: 0x03e900005b9e
Crash State:
  net::cookie_util::GetCookieDomainWithString
  net::GetCookieDomain
  net::CanonicalCookie::Create
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan&range=409082:409160

Minimized Testcase (0.10 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv96TEqjbm2H_s_H90qmwdR7s13hhuufU0kzezlGeSOU7Hh6AcB3ummYg_3L_8AYUgha5dOURKID5yGWkYE9wUlQFqUqWbCt7nAjABKHFIfYPCkI8a_i1yfvg-mg8KuMs5P7fH7GhgYZh5ndEFwqg35pftXnlVQ?testcase_id=5502965303738368
HTTP 308
Location://\.\P0/1/h:0 OD

nvol: no-sthTTP: 307
Set-Cookie: foo;Domain=.;T�P
S;Max;-Ageir e


Filer: mummareddy

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.
Project Member

Comment 6 by ClusterFuzz, Aug 3 2016

ClusterFuzz has detected this issue as fixed in range 409160:409173.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5502965303738368

Fuzzer: libfuzzer_net_url_request_fuzzer
Job Type: libfuzzer_chrome_asan
Platform Id: linux

Crash Type: UNKNOWN
Crash Address: 0x03e900005b9e
Crash State:
  net::cookie_util::GetCookieDomainWithString
  net::GetCookieDomain
  net::CanonicalCookie::Create
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan&range=409082:409160
Fixed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan&range=409160:409173

Minimized Testcase (0.10 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv96TEqjbm2H_s_H90qmwdR7s13hhuufU0kzezlGeSOU7Hh6AcB3ummYg_3L_8AYUgha5dOURKID5yGWkYE9wUlQFqUqWbCt7nAjABKHFIfYPCkI8a_i1yfvg-mg8KuMs5P7fH7GhgYZh5ndEFwqg35pftXnlVQ?testcase_id=5502965303738368
HTTP 308
Location://\.\P0/1/h:0 OD

nvol: no-sthTTP: 307
Set-Cookie: foo;Domain=.;T�P
S;Max;-Ageir e


See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 7 by ClusterFuzz, Aug 31 2016

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5047985081942016

Fuzzer: libfuzzer_net_url_request_fuzzer
Job Type: libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: UNKNOWN
Crash Address: 0x03e900004dfe
Crash State:
  net::cookie_util::GetCookieDomainWithString
  net::GetCookieDomain
  net::CanonicalCookie::Create
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan_debug&range=395717:395804

Minimized Testcase (0.09 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv95y_1Wr1wrYu9T__9dD1nvZsy6QwnipbNKKys212ctvVJzlcT11hYsXzrfnENf7Ggv-JlOSfZ_s_JVa1jef7Da7KKkaT7IyCnIBaIZOaBm3XdLRWATrbOjzZRiZdOQ3qQcLc4nGowCsDisdCrfRCjbotWZhtg?testcase_id=5047985081942016
HTTP 308
Location://.:0\/\P OD

Location: http:/
Set-Cookie: foo=bar�doo/o	!Max;*Ageir"e


See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.

Comment 8 by mmoroz@chromium.org, Aug 31 2016

Cc: mmoroz@chromium.org kcc@chromium.org aizatsky@chromium.org

Comment 9 by droger@chromium.org, Aug 31 2016

Sorry I missed this somehow.

This is only a DCHECK so it's unclear if this bug has any real impact.

I could repro the bug using the repro case in comment #7 and will look at it.
Cc: jww@chromium.org mkwst@chromium.org
So, cookie_util::GetCookieDomainWithString is called with the url: http://.:0///P%20OD
and an empty domain.

url_host ends up being . (the dot character)
which fails the DCHECK(DomainIsHostOnly) and crashes immediately on line 50.

Adding cookie owners who may know what this means.
Cc: -mkwst@chromium.org mmenke@chromium.org
adding mmenke too, who wrote the URLRequest fuzzer.

Cc: mkwst@chromium.org
Cc: droger@chromium.org
Owner: mmenke@chromium.org
This....is interesting.  The problem is the domain name starts with ".".  This isn't actually a valid domain name, but we don't have anything that stops the invalid domain name from making it all the way down to the socket layer, and since the test fixture mocks out DNS lookups, the DNS lookup succeeds.  The cookie store can't handle host names that are weird in that fashion (And, in fact, the cookie spec itself doesn't handle them).

It's unclear to me if a hosts file on any platform could have a working entry for "." or ".blah".

So...We could fix it by making the mock DNS resolver make requests for URLs starting with "." fail, we could make the *real* DNS resolver explicitly do that, we could make the cookie store ignore cookies for such URLs, or we could make URLRequestHttpJob reject such URLs.  So many choices, for a really silly issue we'll probably never run into in the field.
Cc: eroman@chromium.org juliatut...@chromium.org
[+eroman, +juliatuttle]:  After a bit more digging:

Leading dots in URLs violate a fundamental assumption of the cookie spec (Not just our implementation of the spec).

Our built-in HOSTS parser does allow leading/trailing dots in host names.  According to man pages, those aren't allowed.  I suggest we just ignore entries leading dots (I'm not too concerned about trailing ones).

It seems sufficiently weird a case that I think we should also make the HostResolver reject domains with leading dots out of hand.  The CookieStore gets badly confused in that case, and the omnibox actually just removes leading domain name dots from HTTP URLs (And treats http://./ as an invalid URL), so not like we're really supporting them, anyways.  Could imagine GetAddrInfo allowing them on some platform, and weird HOSTS files, so just seems simplest to disallow them at the DNS layer.

Current state isn't all that concerning, just thinking it's simplest to avoid this case entirely, rather than worry about what the cookie store does if it can possibly happen on any supported platform.
Project Member

Comment 15 by bugdroid1@chromium.org, Sep 9 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d6e9161aff5787fa445cbbbdf8dbc48ac95d38ac

commit d6e9161aff5787fa445cbbbdf8dbc48ac95d38ac
Author: mmenke <mmenke@chromium.org>
Date: Fri Sep 09 21:55:44 2016

MockHostResolver:  Don't resolve some types of invalid DNS addresses.

The MockHostResolver didn't have the DNS validity check that
HostResolverImpl had. This was causing a fuzzer to send invalid domains
to the CookieStore code, causing DCHECKs.

Also prevent invalid DNS addresses from being put in the HOSTS list,
when using the internal resolver, as that list is checked before
checking if DNS addresses are valid.

BUG= 632606 

Review-Url: https://codereview.chromium.org/2294343003
Cr-Commit-Position: refs/heads/master@{#417723}

[modify] https://crrev.com/d6e9161aff5787fa445cbbbdf8dbc48ac95d38ac/net/dns/dns_hosts.cc
[modify] https://crrev.com/d6e9161aff5787fa445cbbbdf8dbc48ac95d38ac/net/dns/dns_hosts_unittest.cc
[modify] https://crrev.com/d6e9161aff5787fa445cbbbdf8dbc48ac95d38ac/net/dns/dns_util.cc
[modify] https://crrev.com/d6e9161aff5787fa445cbbbdf8dbc48ac95d38ac/net/dns/dns_util.h
[modify] https://crrev.com/d6e9161aff5787fa445cbbbdf8dbc48ac95d38ac/net/dns/host_resolver_impl.cc
[modify] https://crrev.com/d6e9161aff5787fa445cbbbdf8dbc48ac95d38ac/net/dns/mock_host_resolver.cc

Status: Fixed (was: Assigned)
This should (hopefully) be fixed now.
Project Member

Comment 17 by ClusterFuzz, Sep 10 2016

ClusterFuzz has detected this issue as fixed in range 417609:417727.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5047985081942016

Fuzzer: libfuzzer_net_url_request_fuzzer
Job Type: libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: UNKNOWN
Crash Address: 0x03e900004dfe
Crash State:
  net::cookie_util::GetCookieDomainWithString
  net::GetCookieDomain
  net::CanonicalCookie::Create
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan_debug&range=395717:395804
Fixed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan_debug&range=417609:417727

Minimized Testcase (0.09 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv95y_1Wr1wrYu9T__9dD1nvZsy6QwnipbNKKys212ctvVJzlcT11hYsXzrfnENf7Ggv-JlOSfZ_s_JVa1jef7Da7KKkaT7IyCnIBaIZOaBm3XdLRWATrbOjzZRiZdOQ3qQcLc4nGowCsDisdCrfRCjbotWZhtg?testcase_id=5047985081942016
HTTP 308
Location://.:0\/\P OD

Location: http:/
Set-Cookie: foo=bar�doo/o	!Max;*Ageir"e


See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 18 by sheriffbot@chromium.org, Nov 22 2016

Labels: -Restrict-View-EditIssue
Removing EditIssue view restrictions from ClusterFuzz filed bugs. If you believe that this issue should still be restricted, please reapply the label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -ClusterFuzz-Wrong
We have made a bunch of changes on ClusterFuzz side, so resetting ClusterFuzz-Wrong label.

Sign in to add a comment