[Hunspell Fuzzer] Restrict fuzzer data to valid UTF8

[Hunspell Fuzzer] Restrict fuzzer data to valid UTF8

Hunspell cannot handle invalid UTF8. Chrome makes sure that all
data is in valid UTF8 - this CL enforces the same for the fuzzer.

BUG=none
R=mmoroz@chromium.org

Committed: https://crrev.com/5d6411a1a10652b1a3d9ced8d744f7e427918805
Cr-Commit-Position: refs/heads/master@{#414878}

[mmoroz, 2016-08-09 04:44:30]

LGTM

Thank you groby@ for adding this and for fixing the typo as well :)

[mmoroz, 2016-08-09 04:45:01]

The CQ bit was checked by mmoroz@chromium.org

[commit-bot: I haz the power, 2016-08-09 04:54:22]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-09 04:59:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-09 04:59:21]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[inferno, 2016-08-18 21:50:59]

The CQ bit was checked by inferno@chromium.org

[commit-bot: I haz the power, 2016-08-18 21:51:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kcc2, 2016-08-18 21:52:11]

kcc@chromium.org changed reviewers:
+ kcc@chromium.org

[kcc2, 2016-08-18 21:52:12]

https://codereview.chromium.org/2223603002/diff/1/third_party/hunspell/fuzz/h...
File third_party/hunspell/fuzz/hunspell_fuzzer.cc (right):

https://codereview.chromium.org/2223603002/diff/1/third_party/hunspell/fuzz/h...
third_party/hunspell/fuzz/hunspell_fuzzer.cc:25: // Chromium does - convert to
UTF16, and back to UTF8. Valid UTF8 guaranteed.
Where is the guarantee that Chromium always does that?

[commit-bot: I haz the power, 2016-08-18 21:56:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-18 21:56:27]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[groby-ooo-7-16, 2016-08-18 22:11:14]

On 2016/08/18 21:52:12, kcc2 wrote:
>
https://codereview.chromium.org/2223603002/diff/1/third_party/hunspell/fuzz/h...
> File third_party/hunspell/fuzz/hunspell_fuzzer.cc (right):
> 
>
https://codereview.chromium.org/2223603002/diff/1/third_party/hunspell/fuzz/h...
> third_party/hunspell/fuzz/hunspell_fuzzer.cc:25: // Chromium does - convert to
> UTF16, and back to UTF8. Valid UTF8 guaranteed.
> Where is the guarantee that Chromium always does that?

There is none, except for engineers making sure they actually feed valid data to
third-party APIs. I honestly wouldn't know how to guarantee APIs are always used
correctly - if you do, I'm happy to take suggestions.

Ultimately, Hunspell is fundamentally broken with invalid UTF8 (i.e valid UTF8
is a necessary precondition for using hunspell, otherwise behavior is
undefined). Having a fuzzer that uses invalid UTF8 just proves that point, but
doesn't help in any way discovering meaningful bugs. We're not going to make all
of hunspell deal with invalid encodings.

If you'd like to be more diligent, you could add a fuzzer for HunspellEngine in
Chromium instead, which is the only valid entry point. (Which also can't
guaranteed, but it'd be at least odd to add a second one)

[groby-ooo-7-16, 2016-08-18 22:13:49]

On 2016/08/18 22:11:14, groby wrote:
> On 2016/08/18 21:52:12, kcc2 wrote:
> >
>
https://codereview.chromium.org/2223603002/diff/1/third_party/hunspell/fuzz/h...
> > File third_party/hunspell/fuzz/hunspell_fuzzer.cc (right):
> > 
> >
>
https://codereview.chromium.org/2223603002/diff/1/third_party/hunspell/fuzz/h...
> > third_party/hunspell/fuzz/hunspell_fuzzer.cc:25: // Chromium does - convert
to
> > UTF16, and back to UTF8. Valid UTF8 guaranteed.
> > Where is the guarantee that Chromium always does that?
> 
> There is none, except for engineers making sure they actually feed valid data
to
> third-party APIs. I honestly wouldn't know how to guarantee APIs are always
used
> correctly - if you do, I'm happy to take suggestions.
> 
> Ultimately, Hunspell is fundamentally broken with invalid UTF8 (i.e valid UTF8
> is a necessary precondition for using hunspell, otherwise behavior is
> undefined). Having a fuzzer that uses invalid UTF8 just proves that point, but
> doesn't help in any way discovering meaningful bugs. We're not going to make
all
> of hunspell deal with invalid encodings.
> 
> If you'd like to be more diligent, you could add a fuzzer for HunspellEngine
in
> Chromium instead, which is the only valid entry point. (Which also can't
> guaranteed, but it'd be at least odd to add a second one)

Also: Can't land yet - dependency chains are broken :(  I'll investigate when
I'm back in office (Fri or Mon). Likely, we only need to allow base/ dependency
for the fuzzer subdir, but I don't know if we're OK with that.

[kcc2, 2016-08-18 22:15:00]

On 2016/08/18 22:11:14, groby wrote:
> On 2016/08/18 21:52:12, kcc2 wrote:
> >
>
https://codereview.chromium.org/2223603002/diff/1/third_party/hunspell/fuzz/h...
> > File third_party/hunspell/fuzz/hunspell_fuzzer.cc (right):
> > 
> >
>
https://codereview.chromium.org/2223603002/diff/1/third_party/hunspell/fuzz/h...
> > third_party/hunspell/fuzz/hunspell_fuzzer.cc:25: // Chromium does - convert
to
> > UTF16, and back to UTF8. Valid UTF8 guaranteed.
> > Where is the guarantee that Chromium always does that?
> 
> There is none, except for engineers making sure they actually feed valid data
to
> third-party APIs. I honestly wouldn't know how to guarantee APIs are always
used
> correctly - if you do, I'm happy to take suggestions.

I would suspect that you can restrict the usage of hunspell to a single entry
point 
in chromium (with some GN magic) and then only fuzz that entry point. 

> 
> Ultimately, Hunspell is fundamentally broken with invalid UTF8 (i.e valid UTF8
> is a necessary precondition for using hunspell, otherwise behavior is
> undefined). Having a fuzzer that uses invalid UTF8 just proves that point, but
> doesn't help in any way discovering meaningful bugs. We're not going to make
all
> of hunspell deal with invalid encodings.
> 
> If you'd like to be more diligent, you could add a fuzzer for HunspellEngine
in
> Chromium instead, which is the only valid entry point. (Which also can't
> guaranteed, but it'd be at least odd to add a second one)

Precisely! That would make A LOT of sense, yes!

[groby-ooo-7-16, 2016-08-18 22:16:26]

On 2016/08/18 22:15:00, kcc2 wrote:
> On 2016/08/18 22:11:14, groby wrote:
> > On 2016/08/18 21:52:12, kcc2 wrote:
> > >
> >
>
https://codereview.chromium.org/2223603002/diff/1/third_party/hunspell/fuzz/h...
> > > File third_party/hunspell/fuzz/hunspell_fuzzer.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2223603002/diff/1/third_party/hunspell/fuzz/h...
> > > third_party/hunspell/fuzz/hunspell_fuzzer.cc:25: // Chromium does -
convert
> to
> > > UTF16, and back to UTF8. Valid UTF8 guaranteed.
> > > Where is the guarantee that Chromium always does that?
> > 
> > There is none, except for engineers making sure they actually feed valid
data
> to
> > third-party APIs. I honestly wouldn't know how to guarantee APIs are always
> used
> > correctly - if you do, I'm happy to take suggestions.
> 
> I would suspect that you can restrict the usage of hunspell to a single entry
> point 
> in chromium (with some GN magic) and then only fuzz that entry point. 
> 
> > 
> > Ultimately, Hunspell is fundamentally broken with invalid UTF8 (i.e valid
UTF8
> > is a necessary precondition for using hunspell, otherwise behavior is
> > undefined). Having a fuzzer that uses invalid UTF8 just proves that point,
but
> > doesn't help in any way discovering meaningful bugs. We're not going to make
> all
> > of hunspell deal with invalid encodings.
> > 
> > If you'd like to be more diligent, you could add a fuzzer for HunspellEngine
> in
> > Chromium instead, which is the only valid entry point. (Which also can't
> > guaranteed, but it'd be at least odd to add a second one)
> 
> Precisely! That would make A LOT of sense, yes!

I don't think I currently have the bandwidth to do that. Anybody on your team to
help out?

[kcc2, 2016-08-18 22:37:35]

> I don't think I currently have the bandwidth to do that. Anybody on your team
to
> help out?

Not on my team (Mike is OOO, I am not even Chromium committer, haven't seen this
code before).
Maybe someone wants to do it as a part of the fuzzathon? 
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/MAiBRTllPuI

In general, my goal is not to write fuzzers for every part of Google software
myself, 
but to encourage code owners to do that (scales much better :)

[groby-ooo-7-16, 2016-08-26 01:20:53]

The CQ bit was checked by groby@chromium.org

[commit-bot: I haz the power, 2016-08-26 01:21:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-26 01:26:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-26 01:26:05]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[groby-ooo-7-16, 2016-08-26 01:42:41]

The CQ bit was checked by groby@chromium.org to run a CQ dry run

[groby-ooo-7-16, 2016-08-26 01:42:50]

Description was changed from

==========
[Hunspell Fuzzer] Restrict fuzzer data to valid UTF8

Hunspell cannot handle invalid UTF8. Chrome makes sure that all
data is in valid UTF8 - this CL enforces the same for the fuzzer.

BUG=none
R=mmoroz@chromium.org
==========

to

==========
[Hunspell Fuzzer] Restrict fuzzer data to valid UTF8

Hunspell cannot handle invalid UTF8. Chrome makes sure that all
data is in valid UTF8 - this CL enforces the same for the fuzzer.

BUG=none
R=mmoroz@chromium.org
==========

[groby-ooo-7-16, 2016-08-26 01:42:51]

groby@chromium.org changed reviewers:
+ thestig@chromium.org

[commit-bot: I haz the power, 2016-08-26 01:43:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[groby-ooo-7-16, 2016-08-26 01:43:26]

+thestig for base/ approval to DEPSify them.

inferno/kcc2: Are you OK w/ this landing?

[kcc2, 2016-08-26 01:45:27]

lgtm

It's better to have this fuzz target fixed this way than
to have it broken.

[Lei Zhang, 2016-08-26 01:47:00]

lgtm

[commit-bot: I haz the power, 2016-08-26 01:47:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-26 01:48:00]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[groby-ooo-7-16, 2016-08-26 02:19:11]

The CQ bit was checked by groby@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-26 02:19:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-26 04:35:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-26 04:35:29]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[groby-ooo-7-16, 2016-08-26 23:07:27]

The CQ bit was checked by groby@chromium.org

[groby-ooo-7-16, 2016-08-26 23:07:28]

The patchset sent to the CQ was uploaded after l-g-t-m from mmoroz@chromium.org,
thestig@chromium.org, kcc@chromium.org
Link to the patchset: https://codereview.chromium.org/2223603002/#ps40001
(title: "Clearly, adding a dependency is HARD.")

[commit-bot: I haz the power, 2016-08-26 23:07:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-27 00:25:57]

Description was changed from

==========
[Hunspell Fuzzer] Restrict fuzzer data to valid UTF8

Hunspell cannot handle invalid UTF8. Chrome makes sure that all
data is in valid UTF8 - this CL enforces the same for the fuzzer.

BUG=none
R=mmoroz@chromium.org
==========

to

==========
[Hunspell Fuzzer] Restrict fuzzer data to valid UTF8

Hunspell cannot handle invalid UTF8. Chrome makes sure that all
data is in valid UTF8 - this CL enforces the same for the fuzzer.

BUG=none
R=mmoroz@chromium.org
==========

[commit-bot: I haz the power, 2016-08-27 00:25:58]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-27 00:27:28]

Description was changed from

==========
[Hunspell Fuzzer] Restrict fuzzer data to valid UTF8

Hunspell cannot handle invalid UTF8. Chrome makes sure that all
data is in valid UTF8 - this CL enforces the same for the fuzzer.

BUG=none
R=mmoroz@chromium.org
==========

to

==========
[Hunspell Fuzzer] Restrict fuzzer data to valid UTF8

Hunspell cannot handle invalid UTF8. Chrome makes sure that all
data is in valid UTF8 - this CL enforces the same for the fuzzer.

BUG=none
R=mmoroz@chromium.org

Committed: https://crrev.com/5d6411a1a10652b1a3d9ced8d744f7e427918805
Cr-Commit-Position: refs/heads/master@{#414878}
==========

[commit-bot: I haz the power, 2016-08-27 00:27:29]

Patchset 3 (id:??) landed as
https://crrev.com/5d6411a1a10652b1a3d9ced8d744f7e427918805
Cr-Commit-Position: refs/heads/master@{#414878}