port of sqlite3 fuzzer with some updates

port of sqlite3 fuzzer with some updates

R=aizatsky@chromium.org, inferno@chromium.org, kcc@chromium.org, krasin@chromium.org
BUG=569578
Committed: https://crrev.com/b5a889360ccf53d7da52b37e3dbc1f9d4e335292
Cr-Commit-Position: refs/heads/master@{#366983}

[aizatsky, 2015-12-23 21:04:33]

lgtm

https://codereview.chromium.org/1549793002/diff/1/testing/libfuzzer/fuzzers/s...
File testing/libfuzzer/fuzzers/sqlite3_prepare_v2_fuzzer.cc (right):

https://codereview.chromium.org/1549793002/diff/1/testing/libfuzzer/fuzzers/s...
testing/libfuzzer/fuzzers/sqlite3_prepare_v2_fuzzer.cc:22: if (data[0] & 1)
I suggest you add a comment at the top about how you use data.

[inferno, 2015-12-26 22:33:07]

lgtm with nits, can land/cq after fixing these.

https://codereview.chromium.org/1549793002/diff/1/testing/libfuzzer/fuzzers/s...
File testing/libfuzzer/fuzzers/sqlite3_prepare_v2_fuzzer.cc (right):

https://codereview.chromium.org/1549793002/diff/1/testing/libfuzzer/fuzzers/s...
testing/libfuzzer/fuzzers/sqlite3_prepare_v2_fuzzer.cc:22: if (data[0] & 1)
nit: +1 to Mike's comment. I also suggest putting data[0] into a local since it
is used at two places and makes it easier to read.

https://codereview.chromium.org/1549793002/diff/1/testing/libfuzzer/fuzzers/s...
testing/libfuzzer/fuzzers/sqlite3_prepare_v2_fuzzer.cc:28: int r =
sqlite3_prepare_v2(db, (const char*)(data + 1), size - 1,
s/r/result

https://codereview.chromium.org/1549793002/diff/1/testing/libfuzzer/fuzzers/s...
testing/libfuzzer/fuzzers/sqlite3_prepare_v2_fuzzer.cc:31: for (int i = 0; i <
(data[0] >> 1); i++) {
What is reason for >> 1, also use the local defined above ?

[mmoroz, 2015-12-28 16:13:44]

The CQ bit was checked by mmoroz@chromium.org

[mmoroz, 2015-12-28 16:13:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
aizatsky@chromium.org, inferno@chromium.org
Link to the patchset: https://codereview.chromium.org/1549793002/#ps20001
(title: "fix the nits")

[commit-bot: I haz the power, 2015-12-28 16:14:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1549793002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1549793002/20001

[commit-bot: I haz the power, 2015-12-28 16:49:31]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-12-28 16:50:49]

Description was changed from

==========
port of sqlite3 fuzzer with some updates

R=aizatsky@chromium.org, inferno@chromium.org, kcc@chromium.org,
krasin@chromium.org
BUG=569578
==========

to

==========
port of sqlite3 fuzzer with some updates

R=aizatsky@chromium.org, inferno@chromium.org, kcc@chromium.org,
krasin@chromium.org
BUG=569578

Committed: https://crrev.com/b5a889360ccf53d7da52b37e3dbc1f9d4e335292
Cr-Commit-Position: refs/heads/master@{#366983}
==========

[commit-bot: I haz the power, 2015-12-28 16:50:50]

Patchset 2 (id:??) landed as
https://crrev.com/b5a889360ccf53d7da52b37e3dbc1f9d4e335292
Cr-Commit-Position: refs/heads/master@{#366983}

[kcc2, 2016-01-04 17:26:26]

fuzzing sql is *much* more efficient with a dictionary, which you can grab from
AFL (testcases/_extras/sql.dict)

[kcc2, 2016-01-04 19:05:13]

https://codereview.chromium.org/1549793002/diff/20001/testing/libfuzzer/fuzze...
File testing/libfuzzer/fuzzers/sqlite3_prepare_v2_fuzzer.cc (right):

https://codereview.chromium.org/1549793002/diff/20001/testing/libfuzzer/fuzze...
testing/libfuzzer/fuzzers/sqlite3_prepare_v2_fuzzer.cc:23: int selector =
data[0];
Two problems with this approach: 
1. Now you can not feed the units to standard sqlite utilities, 
you will first need to remove the first byte. 

2. In cases there is a buffer overflow by one byte to the left 
when reading the original buffer, asan will not find it. 
I'd say this is highly unlikely in this particular case though. 

I don't say you should do something about this,
just want to leave this comment FTR.

[mmoroz, 2016-01-15 19:08:16]

https://codereview.chromium.org/1549793002/diff/1/testing/libfuzzer/fuzzers/s...
File testing/libfuzzer/fuzzers/sqlite3_prepare_v2_fuzzer.cc (right):

https://codereview.chromium.org/1549793002/diff/1/testing/libfuzzer/fuzzers/s...
testing/libfuzzer/fuzzers/sqlite3_prepare_v2_fuzzer.cc:22: if (data[0] & 1)
On 2015/12/23 21:04:33, aizatsky wrote:
> I suggest you add a comment at the top about how you use data.

Done.

https://codereview.chromium.org/1549793002/diff/1/testing/libfuzzer/fuzzers/s...
testing/libfuzzer/fuzzers/sqlite3_prepare_v2_fuzzer.cc:28: int r =
sqlite3_prepare_v2(db, (const char*)(data + 1), size - 1,
On 2015/12/26 22:33:07, inferno wrote:
> s/r/result

Done.

https://codereview.chromium.org/1549793002/diff/1/testing/libfuzzer/fuzzers/s...
testing/libfuzzer/fuzzers/sqlite3_prepare_v2_fuzzer.cc:31: for (int i = 0; i <
(data[0] >> 1); i++) {
On 2015/12/26 22:33:07, inferno wrote:
> What is reason for >> 1, also use the local defined above ?

We used least significant bit above for sqlite3_progress_handler selection. Then
we should remove this bit, I think. Otherwise, we always will use loop for odd
number of iterations in cases when progress_handler is used, and always use loop
for even number of iterations in cases without progress_handler.

https://codereview.chromium.org/1549793002/diff/20001/testing/libfuzzer/fuzze...
File testing/libfuzzer/fuzzers/sqlite3_prepare_v2_fuzzer.cc (right):

https://codereview.chromium.org/1549793002/diff/20001/testing/libfuzzer/fuzze...
testing/libfuzzer/fuzzers/sqlite3_prepare_v2_fuzzer.cc:23: int selector =
data[0];
On 2016/01/04 19:05:13, kcc2 wrote:
> Two problems with this approach: 
> 1. Now you can not feed the units to standard sqlite utilities, 
> you will first need to remove the first byte. 
> 
> 2. In cases there is a buffer overflow by one byte to the left 
> when reading the original buffer, asan will not find it. 
> I'd say this is highly unlikely in this particular case though. 
> 
> I don't say you should do something about this,
> just want to leave this comment FTR. 

Thanks,

1. Yes, I do remove first byte at line #35.

2. That's a good point.

We can copy the data without first byte to a new buffer, but as READ
stack-overflow to the left is not likely to happen, it would be redundant
overhead. One more suggestion is to use some XOR'ed value of 3 or 4 different
bytes from data and don't remove anything from the input. It's not perfect
solution in terms of mathematics, but probably will not cause any dependencies
and non-covered cases.