Author: dcheng
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src//+/dfec07b7ec31636c9c160434d203ec40899e7f02
Time: Fri Jun 03 20:26:06 2016
The CL last changed line 202 of file http_request_headers.cc, which is stack frame 5.

@dcheng: Request you to please take a look into it. Please help us to reassign if not with respect to your change.

Thanks.!

ClusterFuzz has detected this issue as fixed in range 408878:408881.

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

Fuzzer: libfuzzer_net_http_proxy_client_socket_fuzzer
Job Type: libfuzzer_chrome_asan
Platform Id: linux

Crash Type: UNKNOWN
Crash Address: 0x03e90000310b
Crash State:
  base::StringValue::StringValue
  base::ListValue::AppendString
  net::HttpRequestHeaders::NetLogCallback
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan&range=408652:408744
Fixed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan&range=408878:408881

Minimized Testcase (0.69 Kb): https://cluster-fuzz.appspot.com/download/AMIfv97yN4kq74OT0lBxkmu3ZJuGyUNqDxZVl5-fGnlcOsj7uG2Ikx3-E4031NYyxGUMxLdnb3qoh1A5ykFUQYbdyDPVj2ujT4JRtQpDCwcc4vJzrzBt3Bx8iXRCdnXXqU7BVDKXCs8JFw61qtd8zDdFN5-echh2ew?testcase_id=4607541321662464

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.

ClusterFuzz testcase is verified as fixed, closing issue.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

Today i am seeing this crash, so reopening the bug.

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

Fuzzer: libfuzzer_net_http_proxy_client_socket_fuzzer
Job Type: libfuzzer_chrome_asan
Platform Id: linux

Crash Type: UNKNOWN
Crash Address: 0x03e900000dfb
Crash State:
  base::StringValue::StringValue
  base::ListValue::AppendString
  net::HttpRequestHeaders::NetLogCallback
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan&range=409082:409160

Minimized Testcase (0.23 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv95h_PYPTj4UZkIanNf5_-utBFtRhHHSFj9ylGgQR2JFDpYR3c5p_3nOM-mIbHcK9xcF5ETyfh9UaVV3frIEsbfRo6K-YxMw8bnS0OntYlqU599UesLEinU93DeBh9kKJ7Dpt7K5tkCL8z8MAFiIlwc1jy3ePA?testcase_id=5082985953230848
HTTP/5.0 407
Proxy-AuthHTHTTP 407
Proxy-Authenticate; digest* realm=(T( 7:Coent�Le/1.8: 5enticatHTTP 407
Proxy-Authenticate:Digest nonce=0Cchangee:00contentsDg*est,realm=*H�TTP� 40Basexceptio5n.ic
Content-Length: 0

 HTHTTP *HTTP_7
 '


Filer: mummareddy

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

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

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

Fuzzer: libfuzzer_net_http_proxy_client_socket_fuzzer
Job Type: libfuzzer_chrome_asan
Platform Id: linux

Crash Type: UNKNOWN
Crash Address: 0x03e900000dfb
Crash State:
  base::StringValue::StringValue
  base::ListValue::AppendString
  net::HttpRequestHeaders::NetLogCallback
  
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.23 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv95h_PYPTj4UZkIanNf5_-utBFtRhHHSFj9ylGgQR2JFDpYR3c5p_3nOM-mIbHcK9xcF5ETyfh9UaVV3frIEsbfRo6K-YxMw8bnS0OntYlqU599UesLEinU93DeBh9kKJ7Dpt7K5tkCL8z8MAFiIlwc1jy3ePA?testcase_id=5082985953230848
HTTP/5.0 407
Proxy-AuthHTHTTP 407
Proxy-Authenticate; digest* realm=(T( 7:Coent�Le/1.8: 5enticatHTTP 407
Proxy-Authenticate:Digest nonce=0Cchangee:00contentsDg*est,realm=*H�TTP� 40Basexceptio5n.ic
Content-Length: 0

 HTHTTP *HTTP_7
 '


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.

I don't think this should have been assigned to me. It's apparently fixed, but I can't really see a good reason for why this regressed / why it got fixed. This should really be assigned to a net owner (since that's likely where the problem exists: the net code is probably passing a bad string into AppendString().

Moving this to untriaged will make it show up on the net triagers queue. You're right it does look like a net issue.

This just seems to be an issue with non-UTF8 headers. We should probably be canonicalizing the strings before trying to print them.

A smaller test case is attached.

Deleted: fuzz 83 bytes

fuzz 83 bytes View Download

There's no such thing as canonicalizing headers, though.  We get bytes, and if they're not ASCII, we have no idea why they are.

This is the frustrating thing about JSON, it doesn't support binary blobs :(

base::StringValue assumes a unicode string. So I expect this is failing now since DCHECKs were enabled on fuzztests.

As Matt points out, there is no guarantee that headers or other values we are passing as std::string are UTF-8 encoded, so this isn't something we can simply fix by passing in different data.

From base::Value's abstraction we could "fix" this by the data as base::BinaryValue. I think that would fix the immediate problem, but it also a terrible solution in its own right:
  - Basically all places we pass "strings" from net need to be base::BinaryValue since the strings are primarily a bag of bytes
  - This will break JSON serialization

I don't believe the JSON serializer can handle base::BinaryValue (and in fact how would it?). We could address this by base64 encoding all the BinaryValues into string value, but that is also pretty terrible, and puts into question the usefulness of using JSON as the serialization format of NetLog dumps to begin with.

Hmm.

We could just escape all values >= 0x80, I guess, or just do that for characters that don't look to be UTF-8.

Does HTTP2 specify character encoding?

Err yeah, I meant escaping them for printing, though it needs to be in some consistent format that anything that wants to parse the values can understand. JSON will fail spectacularly at binary values in lots of places.

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

Fuzzer: libfuzzer_net_http_proxy_client_socket_fuzzer
Job Type: libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: UNKNOWN
Crash Address: 0x03e900005af8
Crash State:
  base::StringValue::StringValue
  base::ListValue::AppendString
  net::HttpRequestHeaders::NetLogCallback
  

Minimized Testcase (0.12 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv949lDd4SXk67UwTiSPx94XZSuQbyRy3lTc9YF8V6_Os3aLvY_sktj6EBvLjx4wR_4OgloeriNBmnZfm-I7dcZaLKhc1YL7M_ZBTxjgP6mgwrQWJKP_1QfbgA-QuFrz6k1mLC2vTAOHwXI926zpO_12S3kpPXA?testcase_id=6029530768867328
HTTP/5.0 407
Proxy-authentictTaHTP 407
Proxy-Authenticate:Digest nonce="""""las""""�""""".
Content-Length: 0

 H: Keep-


Additional requirements: Requires Gestures

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

Not sure if this bug related to //net, but, mmenke@, as an author of the fuzzer, would you mind taking a look?

I just don't have time - I have a significant regression on my plate, and little enough free time to look at that.  This issue is already understood (See discussion above), and isn't really an issue in production code (Well, it *is*, but only if you're using net-internals).

Thanks Matt!

ClusterFuzz has detected this issue as fixed in range 419320:419370.

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

Fuzzer: libfuzzer_net_http_proxy_client_socket_fuzzer
Job Type: libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: UNKNOWN
Crash Address: 0x03e900005af8
Crash State:
  base::StringValue::StringValue
  base::ListValue::AppendString
  net::HttpRequestHeaders::NetLogCallback
  
Fixed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan_debug&range=419320:419370

Minimized Testcase (0.12 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv949lDd4SXk67UwTiSPx94XZSuQbyRy3lTc9YF8V6_Os3aLvY_sktj6EBvLjx4wR_4OgloeriNBmnZfm-I7dcZaLKhc1YL7M_ZBTxjgP6mgwrQWJKP_1QfbgA-QuFrz6k1mLC2vTAOHwXI926zpO_12S3kpPXA?testcase_id=6029530768867328
HTTP/5.0 407
Proxy-authentictTaHTP 407
Proxy-Authenticate:Digest nonce="""""las""""�""""".
Content-Length: 0

 H: Keep-


Additional requirements: Requires Gestures

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.

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

Fuzzer: libfuzzer_net_http_proxy_client_socket_fuzzer
Job Type: libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: UNKNOWN
Crash Address: 0x03e900007602
Crash State:
  base::StringValue::StringValue
  base::internal::MakeUniqueResult<base::StringValue>::Scalar base::MakeUnique<bas
  base::ListValue::AppendString
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan_debug&range=419320:419370

Minimized Testcase (0.10 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv96YIVIhU8n6yuxQNdjUZFmLWqhQVoqNpTy8BiP3TR6c6wdefccLPrLXeamT1X_odnEujy0nmOk4ZVD-0T-gj2njabIsrCD1bknncryzef2IbvZ6RHaCZNmsML263UouIXUk5z0oDeVvHe5j5GYzKZhwJz_KoA?testcase_id=4880343008280576
*HTTP/2.5TT'��/ 407
PHTTP 427
Proxy-Authenticate:Digest nonce=8073"�����
Content-Length: 0

5Psi\^


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

Update: I'll find an owner for this -- I think this is a good-scoped starter project for Yixin.

I don't want to have to teach the net-internals javascript about which fields are escaped or base64-encoded, so we will need to carry the "type information" either in the field name (gross for log producer), or in the value itself (implicitly or explicitly) -- also gross.

Matt's proposal in comment #12 of escaping them to valid UTF-8 strings has the advantage of keeping the log format backwards compatible and the log viewer forwards compatible, albeit with some ambiguities on how to interpret the data.

But I am thinking of taking a slightly different approach and tagging the type explicitly in the value (which will mean updating the log viewer to understand it).

For instance in C++ consumers would call something like this when adding non-UTF8 values to the parameters:

  std::unique_ptr<base::Value> MakeBinaryValueForNetLog(data);

Conceptually this creates a base::BinaryValue, but in practice it will probably just build an object that is friendlier to JSON serialization, say:

  {"b64data": "<base64 encoded data as UTF8 string>"}

If in the future if we ever add hooks to the base:: JSON writer we can replace the data model with base::BinaryValue directly and do this conversion at serialization instead.

We have some other base64-encoded fields today which could be upgraded to this format too.

The thing is, these aren't really "binary values", they're generally just values that are supposed to be ASCII text, but aren't because servers are weird.  Displaying them as binary seems worse to me than the most trivial thing we could do - replacing non-UTF8 characters with a simple placeholder.

I am not proposing displaying them as binary -- we would change the log viewer to best-effort display them as text. What I am proposing is encoding them in binary to reflect the reality that the contents are outside of our control.

Some details:

This bug is happening when reflecting the *Request* headers, not the Response headers.

For response headers we currently escape non-ASCII values [1]

For consistency, and to close out this fuzz crash bug, we can do the same thing for Request headers.

We can continue the rest of the discussion on other bugs.


The specific reason why the Request headers (which we construct) have non-ASCII here is because it is reflecting a nonce parsed from request headers. This is probably something to address as well, since there is some awkward parsing behavior (although encodings in general for HTTP auth is all sorts of broken).

The received response headers (non-ASCII, but we escape these when writing ot NetLog):

  "Digest nonce=8073\"\xa8\xa8\xa8\xa8\xa8"

Constructed request header (non-ASCII) that we DCHECK on:

  "Proxy-Authorization: Digest username=\"user\", realm=\"\", nonce=\"8073\\\"\xa8\xa8\xa8\xa8\xa8\", uri=\"foo:80\", response=\"5ce5d7e48d27f45614a13f5004590639\""

[1] https://cs.chromium.org/chromium/src/net/http/http_response_headers.cc?q=HttpResponseHeaders::NetLogCallback&sq=package:chromium&dr=CSs&l=1431

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

commit 396ba79a4314fd0be39f78e1e2ca467951a37cf4
Author: eroman <eroman@chromium.org>
Date: Tue Oct 18 20:45:43 2016

Escape non-ASCII Request header values before attaching them as a NetLog string.

This is a quick fix for a libfuzzer crash.

BUG= 633069 

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

[modify] https://crrev.com/396ba79a4314fd0be39f78e1e2ca467951a37cf4/net/http/http_request_headers.cc

Also filed followup tasks,  issue 657116 ,  issue 657115 .

ClusterFuzz has detected this issue as fixed in range 426010:426066.

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

Fuzzer: libfuzzer_net_http_proxy_client_socket_fuzzer
Job Type: libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: UNKNOWN
Crash Address: 0x03e900007602
Crash State:
  base::StringValue::StringValue
  base::internal::MakeUniqueResult<base::StringValue>::Scalar base::MakeUnique<bas
  base::ListValue::AppendString
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan_debug&range=419320:419370
Fixed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan_debug&range=426010:426066

Minimized Testcase (0.10 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv96YIVIhU8n6yuxQNdjUZFmLWqhQVoqNpTy8BiP3TR6c6wdefccLPrLXeamT1X_odnEujy0nmOk4ZVD-0T-gj2njabIsrCD1bknncryzef2IbvZ6RHaCZNmsML263UouIXUk5z0oDeVvHe5j5GYzKZhwJz_KoA?testcase_id=4880343008280576
*HTTP/2.5TT'��/ 407
PHTTP 427
Proxy-Authenticate:Digest nonce=8073"�����
Content-Length: 0

5Psi\^


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.

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