go-sql-driver / mysql

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

Jump to bottom

Added fuzzer #1097

Merged

julienschmidt merged 5 commits into go-sql-driver:master from AdamKorcz:master

May 21, 2020

+24 −0

Contributor

AdamKorcz commented May 11, 2020

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.

AdamKorcz added 2 commits


          Added fuzzer

a46920b


          Update to package

b8266b2

julienschmidt reviewed

Member

julienschmidt left a comment

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.

Contributor Author

AdamKorcz commented May 16, 2020 •

edited

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.

Member

julienschmidt commented May 17, 2020

Ok sure, we have to start somewhere.

julienschmidt added the testing label

julienschmidt requested changes

Member

julienschmidt left a comment

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


          Updated fuzzer according to maintainers feedback

82ccd1f

julienschmidt reviewed

fuzz.go Outdated Show resolved Hide resolved

AdamKorcz added 2 commits


          gofmt'd fuzzer

bccf78b


          Running Travis again. He seems to be stuck

e66cf24

julienschmidt added this to the v1.6.0 milestone

julienschmidt approved these changes

Member

julienschmidt left a comment

LGTM. Thanks!

julienschmidt merged commit 128a673 into go-sql-driver:master

3 checks passed

3 checks passed

Travis CI - Pull Request Build Passed

Details

WIP Ready for review

Details

coverage/coveralls Coverage increased (+0.07%) to 81.73%

Details

Contributor Author

AdamKorcz commented May 21, 2020

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.

Member

julienschmidt commented May 21, 2020

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?

Contributor Author

AdamKorcz commented May 21, 2020

Sure, I will get it added.

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

Contributor Author

AdamKorcz commented May 22, 2020

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

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

Contributor Author

AdamKorcz commented Jun 3, 2020

@julienschmidt Kind ping regarding the email addresses for bug reports

Member

arnehormann commented Jun 4, 2020

@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.

Contributor Author

AdamKorcz commented Jun 9, 2020

@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 added a commit to tz70s/mysql that referenced this pull request


          Add fuzzer (go-sql-driver#1097)

e0382cc

The fuzzer targets the `Open` function, which can be run locally as well as through oss-fuzz's infrastructure.

tz70s added a commit to tz70s/mysql that referenced this pull request


          Add fuzzer (go-sql-driver#1097)

c1630fc

The fuzzer targets the `Open` function, which can be run locally as well as through oss-fuzz's infrastructure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment