Opened 7 years ago

Closed 7 years ago

#3783 closed patch (fixed)

Setup OSS FUZZ testing for PostGIS

Reported by: robe Owned by: robe
Priority: medium Milestone: PostGIS 2.4.0
Component: postgis Version: master
Keywords: Cc:

Description

Detailed in Even's note sent to mailing list:

https://lists.osgeo.org/pipermail/postgis-devel/2017-July/026216.html

repeated for completeness:

Hi,

I've prototyped an integration of liblwgeom from PostGIS repository with oss-fuzz.

Quoting https://github.com/google/oss-fuzz/ ,
"""
Fuzz testing is a well-known technique for uncovering various kinds of programming errors in software.
Many of these detectable errors (e.g. buffer overflow) can have serious security implications.

We successfully deployed guided in-process fuzzing of Chrome components and found hundreds of
security vulnerabilities and stability bugs. We now want to share the experience and the
service with the open source community.

In cooperation with the Core Infrastructure Initiative, OSS-Fuzz aims to make common
open source software more secure and stable by combining modern fuzzing techniques
and scalable distributed execution.
""""

GDAL and proj.4 have joined oss-fuzz, for a few weeks  and this is really efficient. I've fixed between 300 and 400 bugs in GDAL...

So I just gave it a try with PostGIS, concentrating on liblwgeom, since it builds nicely in oss-fuzz environment
(plain "make" in top repository fails in oss-fuzz from some reason I haven't investigated)
As an example, I've created 2 fuzzers, one for lwgeom_from_wkb() and the other one for lwgeom_from_wkt().
More could be done, based on those examples. Left as an exercice to other developers.

Integration of a software with oss-fuzz is made of 2 parts:
- fuzzer entry points must be in the project repository : https://github.com/rouault/postgis/tree/ossfuzz/fuzzers
- a metadata file (project.yaml), a Dockerfile (download needed packages & PostGIS source code) and
a "bootstrap" build.sh script  must be integrated in OSS-Fuzz own repo too :
https://github.com/rouault/oss-fuzz/tree/postgis/projects/postgis

For now, I've done this in my own postgis and ossz-fuzz git forks as you can see, but ultimately if the
project agrees we should merge this into their respective official repos.

But pending that, you can already try this stuff locally
{{{
Make sure you have Docker installed

git clone  --branch postgis https://github.com/rouault/oss-fuzz
cd oss-fuzz

Build the Docker image:
python infra/helper.py build_image postgis

Build PostGIS and the fuzzer programs with the address sanitizer
python infra/helper.py build_fuzzers --sanitizer address postgis

Run one of the fuzzer (you can try with wkt_import_fuzzer too)
python infra/helper.py run_fuzzer postgis wkb_import_fuzzer
}}}

My local experiments show that lwgeom_from_wkt() seems to be rather robust,
but lwgeom_from_wkb() has a few bugs. For the record, I found and fixed (in my fork) this memory leak
(memory leak = leak in a case where lw_error() is not called) in
https://github.com/rouault/postgis/commit/cf179396b719223653eee56a01189339e0abcc0d
There's also a heap buffer overflow it just detected in
ptarray_from_wkb_state /src/postgis/liblwgeom/lwin_wkb.c:367

So if the community is interested in a closer integration in OSS Fuzz, next steps are :
1) someone with PostGIS commit rights merges https://github.com/rouault/postgis/commit/0181a28ab01764b4e6d11a5d2ffe7edce96498c6
into PostGIS SVN (as well as the bug fix https://github.com/rouault/postgis/commit/cf179396b719223653eee56a01189339e0abcc0d while you are it)
2) interested core PostGIS developers give me a @gmail.com email, so I add it in
https://github.com/rouault/oss-fuzz/blob/postgis/projects/postgis/project.yaml 
This way they will then have access to the bug reports that are embargoed for
90 days (or 30 days after OSS Fuzz has found them to be fixed)
3) I then modify https://github.com/rouault/oss-fuzz/blob/postgis/projects/postgis/Dockerfile to
point to PostGIS official github mirror instead of my fork
4) I then submit a pull request to https://github.com/google/oss-fuzz/ with my
https://github.com/rouault/oss-fuzz postgis branch . They may accept or not the application, but I guess they will accept.
If they don't, you can also play with it locally as I showed above. And this is strongly recommended
to do so when adding a new fuzzer for example.
5) once the project is accepted, monitor https://bugs.chromium.org/p/oss-fuzz/issues/list?q=postgis and fix the bugs !
6) add more fuzzers. Hint: in PostGIS "fuzzers" directory, "make dummyfuzzers" to check that your fuzzer builds.

Note: I don't volunteer to fix all bugs that will be found. I have already enough to do with GDAL... I wouldn't
mind if someone wants to be the declared maintainer in oss-fuzz projects/postgis/project.yaml and projects/postgis/Dockerfile

Note 2: if you look closely at https://github.com/rouault/postgis/blob/ossfuzz/fuzzers/wkb_import_fuzzer.cpp
you will notice that it is a bit messy since it stubs GEOS and geod_ symbols. This is due to the fact
that OSS-Fuzz requires that the fuzzer programs are completely statically linked, and Ubuntu doesn't ship
with static builds of geos (actually just libgeos.a but no libgeos_c.a) as far as I can see.
All this could be improved by adding a download of GEOS source code in
https://github.com/rouault/oss-fuzz/blob/postgis/projects/postgis/Dockerfile and building it manually in
https://github.com/rouault/oss-fuzz/blob/postgis/projects/postgis/build.sh

Thoughts ?

Even

Change History (6)

comment:1 by robe, 7 years ago

Summary: OSS FUZZ entry pointsSetup OSS FUZZ testing for PostGIS

comment:2 by robe, 7 years ago

In 15484:

Commit Fuzzers for liblwgeom wkb OSS FUZZ testing
Fuzzers contributed by Even Rouault
References #3783 for PostGIS 2.4 (trunk)

comment:3 by robe, 7 years ago

Also made changes to the ossfuzz and Even committed and I filled in an additional CLA to cover my edits.

https://github.com/google/oss-fuzz/pull/709

So I guess it's just a matter of waiting for our pull request to be accepted and add more tests to our fuzz folder.

BTW pramsey and strk I took the liberty of adding your names in the cc for project:

https://github.com/google/oss-fuzz/pull/709/commits/163c0c4902f5381338ba303bd5425ddf8bfbb503

so you'll need to do a pull request to take yourselves out :).

comment:4 by robe, 7 years ago

Okay we are officially in now.

https://github.com/google/oss-fuzz/tree/master/projects/postgis

Time to beef up those tests though

and address kcc's comment here:

https://github.com/google/oss-fuzz/pull/709

 projects/postgis/build.sh
+make -j$(nproc) -s
+cd ..
+
+./fuzzers/build_google_oss_fuzzers.sh
@kcc
kcc 5 hours ago Contributor

This should work as is, but it's preferable to have the fuzz targets built as part of the regular developer build (make).
Otherwise the risk is that the fuzz targets will often bit rot and we will see failures only on oss-fuzz

comment:5 by strk, 7 years ago

Let's add a separate target for building fuzzers, and another one to *run* the fuzzers. Then we can have bots run them (would be similar to your garden tests…)

comment:6 by robe, 7 years ago

Resolution: fixed
Status: assignedclosed
Type: defectpatch
Note: See TracTickets for help on using tickets.