Submit the re, json, csv, & struct modules to oss-fuzz testing

[gpshead]

BPO	29505
Nosy	@brettcannon, @terryjreedy, @gpshead, @tiran, @ssbr, @alex, @bitdancer, @ammaraskar, @miss-islington
PRs	bpo-29505: Add fuzz tests for float(str), int(str), unicode(str) #2878 bpo-29505: Avoid UB in test selection macro. #3407 bpo-29505: Fuzz ast.parse() / compile() with PyCF_ONLY_AST #3437 bpo-29505: Fix interpreter in fuzzing targets to be relocatable #13907 [3.8] bpo-29505: Fix interpreter in fuzzing targets to be relocatable (GH-13907) #13914 [3.7] bpo-29505: Fix interpreter in fuzzing targets to be relocatable (GH-13907) #13915 bpo-29505: Fuzz json module, enforce size limit on int(x) fuzz #13991 [3.8] bpo-29505: Fuzz json module, enforce size limit on int(x) fuzz (GH-13991) #14005 [3.7] bpo-29505: Fuzz json module, enforce size limit on int(x) fuzz (GH-13991) #14006 bpo-29505: Add fuzzing for re.compile, re.load and csv.reader #14255 [3.8] bpo-29505: Add more fuzzing for re.compile, re.load and csv.reader (GH-14255) #14478 [3.7] bpo-29505: Add more fuzzing for re.compile, re.load and csv.reader (GH-14255) #14479 bpo-29505: Fuzz struct.unpack and catch RecursionError in re.compile #18679 bpo-29505: Add fuzzer for ast.literal_eval #28777

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/gpshead'
closed_at = <Date 2021-04-02.16:05:39.827>
created_at = <Date 2017-02-08.20:54:21.924>
labels = ['type-security', '3.7']
title = 'Submit the re, json, csv, & struct modules to oss-fuzz testing'
updated_at = <Date 2021-10-06.23:22:19.809>
user = 'https://github.com/gpshead'

bugs.python.org fields:

activity = <Date 2021-10-06.23:22:19.809>
actor = 'miss-islington'
assignee = 'gregory.p.smith'
closed = True
closed_date = <Date 2021-04-02.16:05:39.827>
closer = 'ammar2'
components = []
creation = <Date 2017-02-08.20:54:21.924>
creator = 'gregory.p.smith'
dependencies = []
files = []
hgrepos = []
issue_num = 29505
keywords = ['patch']
message_count = 29.0
messages = ['287363', '287568', '292821', '293263', '293340', '299183', '299207', '301494', '301495', '301530', '301535', '301732', '301759', '301897', '301901', '301918', '301922', '301932', '345039', '345044', '345045', '345304', '345306', '345307', '346916', '346918', '347496', '390076', '403340']
nosy_count = 10.0
nosy_names = ['brett.cannon', 'terry.reedy', 'gregory.p.smith', 'christian.heimes', 'Devin Jeanpierre', 'alex', 'r.david.murray', 'Guido', 'ammar2', 'miss-islington']
pr_nums = ['2878', '3407', '3437', '13907', '13914', '13915', '13991', '14005', '14006', '14255', '14478', '14479', '18679', '28777']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue29505'
versions = ['Python 2.7', 'Python 3.6', 'Python 3.7']

• PR: gh-73691: Increase size limits in _xxtestfuzz #99070

[gpshead]

For reference, read https://github.com/google/oss-fuzz.

We should investigate creating fuzz targets for the Python re module (_sre.c) at a minimum. There are probably other good targets as well such as _json.c and _csv.c.

pickle and marshal are not intended to be secure so ignore those.

[terryjreedy]

It does not appear to me that targets have to be security critical, though that is certainly a good place to start. The Chrome tests found 100s of "security vulnerabilities and stability bugs".

The important thing is that there be someone willing to receive and act on reports. Would 'make public after 90 days' ever be a problem? AFAIK, most Python security issues are already public here on the tracker from day 1.

[ssbr]

Aha, I found an existing issue!

For adding to oss-fuzz, is there a contact email we can use that is connected to a google account? I am tempted to just put gregory.p.smith on there if not. :)

I can volunteer to fuzz some interesting subset of the stdlib. The list I've come up with (by counting uses in my code) is:

the XML parser (which seems to be written in C)
struct (unpack)
the various builtins that parse strings (like int())
hashlib
binascii
datetime's parsing
json

I'd also suggest the ast module, since people do use ast.literal_eval on untrusted strings, but I probably won't do that one myself.

I wrote a fuzz test for json via upstream simplejson, but the bug on github is getting stale: simplejson/simplejson#163

Should I add it to CPython instead?

We should investigate creating fuzz targets for the Python re module (_sre.c) at a minimum.

If we prioritize based on security risk, I'd argue that this is lower priority than things like json's speedup extension module, because people should generally not pass untrusted strings to the re module: it's very easy to DOS a service with regexes unless you're using RE2 or similar -- which is fuzzed. In contrast, json is supposed to accept untrusted input and people do that very often.

(OTOH, I would be willing to bet that fuzzing re will yield more bugs than fuzzing json.)

[gpshead]

you can list me as a oss-fuzz contact. use my work email address.

simplejson is worthy but as both it and the python standard library ship separately people use both so they both ultimately deserve fuzzing and fixing on their own so I'd add it to CPython as well.

[ssbr]

google/oss-fuzz#583 is the PR to oss-fuzz to add the project. I'm working on actual tests to be submitted here.

[terryjreedy]

As I read 583, they are planning to fuzz 3.6. Why not branch master? I think it more likely that we accidentally add a vulnerability to master then that we accidentally close one.

[ssbr]

I think they misspoke, it's normal with fuzzing to test against master. The current draft of the code runs this git pull before building/launching any tests:

git clone --depth 1 https://github.com/python/cpython.git cpython

Speaking of which, I forgot to update this bug thread with the followup PR to actually run CPython's fuzz tests (when they exist): google/oss-fuzz#731. That's where I grabbed the git clone statement from. I think that will be merged after some version of PR 2878 lands in CPython (still in code review / broken).

For Python 2 I guess it's different, and we will test against the 2.7 branch, right?

[gpshead]

New changeset c5bace2 by Gregory P. Smith (Devin Jeanpierre) in branch 'master':
bpo-29505: Add fuzz tests for float(str), int(str), unicode(str) (bpo-2878)
c5bace2

[gpshead]

alright, with that in, feel free to figure out the oss-fuzz configuration side and fire things up Devin. :)

[tiran]

GCC complains about the patch:

/home/heimes/dev/python/cpython/Modules/_xxtestfuzz/fuzzer.c: In function ‘LLVMFuzzerTestOneInput’:
/home/heimes/dev/python/cpython/Modules/_xxtestfuzz/fuzzer.c:109:1: warning: this use of "defined" may not be portable [-Wexpansion-to-defined]
 #if _Py_FUZZ_YES(fuzz_builtin_float)
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/heimes/dev/python/cpython/Modules/_xxtestfuzz/fuzzer.c:109:1: warning: this use of "defined" may not be portable [-Wexpansion-to-defined]
/home/heimes/dev/python/cpython/Modules/_xxtestfuzz/fuzzer.c:112:1: warning: this use of "defined" may not be portable [-Wexpansion-to-defined]
 #if _Py_FUZZ_YES(fuzz_builtin_int)
 ^~~~~~~~~~~~~~~~~~~~~~~~~
/home/heimes/dev/python/cpython/Modules/_xxtestfuzz/fuzzer.c:112:1: warning: this use of "defined" may not be portable [-Wexpansion-to-defined]
/home/heimes/dev/python/cpython/Modules/_xxtestfuzz/fuzzer.c:115:1: warning: this use of "defined" may not be portable [-Wexpansion-to-defined]
 #if _Py_FUZZ_YES(fuzz_builtin_unicode)
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/heimes/dev/python/cpython/Modules/_xxtestfuzz/fuzzer.c:115:1: warning: this use of "defined" may not be portable [-Wexpansion-to-defined]

[ssbr]

Huh. I would not have predicted that.

https://gcc.gnu.org/onlinedocs/cpp/Defined.html

I'll send a fix.

[ssbr]

So here's an interesting issue: oss-fuzz requires that the built location be movable. IOW, we build Python into $OUT, and then the $OUT directory gets moved somewhere else and the fuzz test gets run from there. This causes problems because Python can no longer find where the modules it needs are (encodings for example).

First thought: wouldn't it be nice if we could make a prepackaged and hermetic executable that we can move around freely?

Second thought: isn't that "Hermetic Python", as used within Google?

Third thought: doesn't Google have an internal fuzz testing environment we can use, instead of oss-fuzz?

So unless someone says this is a bad idea, I'd propose we not run these in oss-fuzz and instead run them in Google proper. The alternative is if there's a way to make it easy to move Python around -- is there a way to build it s.t. the import path is relative and so on?