Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added fuzzer #1097

Merged
merged 5 commits into from
May 21, 2020
Merged

Added fuzzer #1097

merged 5 commits into from
May 21, 2020

Conversation

AdamKorcz
Copy link
Contributor

This PR adds a fuzzer in an added /fuzzing directory. The fuzzer targets the Open-function.

The fuzzer can be run locally, and I also managed to run it through oss-fuzz's infrastructure. I suggest integrating Go-MySQL-Driver into oss-fuzz. oss-fuzz will run the fuzzers continuously on their platform, and if a bug is encountered, a report will be sent to the maintainers on the contact list. It is a free service that is offered with an expectation that bugs are fixed, so that the fuzzers can keep running and check for other bugs.

The current fuzzer is a good starting point. I would like to write more fuzzers for Go-MySQL-Driver to progressively increase code coverage and optimize their effectiveness.

If there is interest in integrating with oss-fuzz, I will be happy to do that. All I need are the email addresses to add to the contact list. Please note that this list will be public and can be modified at any time.

Copy link
Member

@julienschmidt julienschmidt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at other projects, I believe this should be just a file with a build tag in the main package. See e.g. https://github.com/golang/go/blob/4ad13555184eb0697c2e92c64c1b0bdb287ccc10/src/html/fuzz.go
This PR would add a new exported sub-package, which is definitely a no-go.

I'm also not sure how much sense it makes to fuzz Open, as it does nothing more than string parsing. It never starts any network IO on its own.

@AdamKorcz
Copy link
Contributor Author

AdamKorcz commented May 16, 2020

This PR would add a new exported sub-package, which is definitely a no-go

Sure, this should not be a problem to amend.

I'm also not sure how much sense it makes to fuzz Open, as it does nothing more than string parsing. It never starts any network IO on its own.

Fair enough, although parsers do cause crashes. This writeup and this as well provide some examples of that.

Looking at other projects, I believe this should be just a file with a build tag in the main package

Yes, this can be added. I can confirm that the fuzzers are running fine on oss-fuzz's infrastructure without it.

@julienschmidt
Copy link
Member

Ok sure, we have to start somewhere.

Copy link
Member

@julienschmidt julienschmidt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Move it to the main package (with build tag)
  • Indicate who the copyright holder is or make an addition to the AUTHORS file

fuzzing/fuzz.go Outdated Show resolved Hide resolved
fuzzing/fuzz.go Outdated Show resolved Hide resolved
fuzzing/fuzz.go Outdated Show resolved Hide resolved
fuzzing/fuzz.go Outdated Show resolved Hide resolved
fuzz.go Outdated Show resolved Hide resolved
@julienschmidt julienschmidt added this to the v1.6.0 milestone May 21, 2020
Copy link
Member

@julienschmidt julienschmidt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@julienschmidt julienschmidt merged commit 128a673 into go-sql-driver:master May 21, 2020
@AdamKorcz
Copy link
Contributor Author

Shall I setup continuous fuzzing for mysql through oss-fuzz as well? I have the fuzzer running on their infrastructure, and I will be happy to set it up.

@julienschmidt
Copy link
Member

We would definitely appreciate it 👍

I just noticed that you didn't indicate who the copyright holder is (simple comment is enough) or made an addition to the AUTHORS file. Could you please also do that?

@AdamKorcz
Copy link
Contributor Author

Sure, I will get it added.

Could you provide me with an email address for potential bug reports?

@AdamKorcz
Copy link
Contributor Author

@julienschmidt On the question of the copyright holder, will the existing header not suffice?

mysql/fuzz.go

Lines 1 to 9 in 128a673

// Go MySQL Driver - A MySQL-Driver for Go's database/sql package.
//
// Copyright 2020 The Go-MySQL-Driver Authors. All rights reserved.
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// You can obtain one at http://mozilla.org/MPL/2.0/.
// +build gofuzz

@AdamKorcz
Copy link
Contributor Author

@julienschmidt Kind ping regarding the email addresses for bug reports

@arnehormann
Copy link
Member

@AdamKorcz there is no common address, afaik. Can you create issues instead? Fuzzing results should (famous last words) not be security critical, the driver is as memory safe as Go is. Reports can be public ... unless a more active maintainer disagrees.

@AdamKorcz
Copy link
Contributor Author

@arnehormann We can add any number of maintainers' email addresses to the list of bug reports - it doesn't have to be a single email address.
Creating issues, I am afraid, would not be a viable solution. For the notifications to be effective and to avoid delay, the maintainers interested in fixing potential bugs should receive the emails directly.

tz70s pushed a commit to tz70s/mysql that referenced this pull request Sep 5, 2020
The fuzzer targets the `Open` function, which can be run locally as well as through oss-fuzz's infrastructure.
tz70s pushed a commit to tz70s/mysql that referenced this pull request Sep 5, 2020
The fuzzer targets the `Open` function, which can be run locally as well as through oss-fuzz's infrastructure.
@dolmen dolmen mentioned this pull request Jun 2, 2023
dolmen added a commit to dolmen/mysql that referenced this pull request Oct 17, 2023
fuzz.go (added in go-sql-driver#1097) uses gofuzz. But in go-sql-driver#1444 I've added a better
fuzzer that uses Go builtin fuzzing.
@dolmen dolmen mentioned this pull request Oct 17, 2023
1 task
dolmen added a commit to dolmen/mysql that referenced this pull request Oct 17, 2023
fuzz.go (added in go-sql-driver#1097) uses gofuzz. But in go-sql-driver#1444 I've added a better
fuzzer that uses Go builtin fuzzing.

Closes go-sql-driver#1445.
dolmen added a commit to dolmen/mysql that referenced this pull request Oct 17, 2023
fuzz.go (added in go-sql-driver#1097) uses gofuzz. But in go-sql-driver#1444 I've added a better
fuzzer that uses Go builtin fuzzing.

Closes go-sql-driver#1445.
methane pushed a commit that referenced this pull request Oct 19, 2023
fuzz.go (added in #1097) uses gofuzz.
But #1444 added a better fuzzer that uses Go builtin fuzzing.

Closes #1445.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants