pelletier / go-toml

Go library for the TOML file format
https://github.com/pelletier/go-toml
Other
1.67k stars 208 forks source link

OSS-Fuzz issue 63789 #913

Open oss-fuzz-robot opened 8 months ago

oss-fuzz-robot commented 8 months ago

OSS-Fuzz has found a bug in this project. Please see https://oss-fuzz.com/testcase?key=5412509395582976 for details and reproducers.

This issue is mirrored from https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=63789 and will auto-close if the status changes there.

If you have trouble accessing this report, please file an issue at https://github.com/google/oss-fuzz/issues/new.

pelletier commented 8 months ago
fuzz_test.go:31: INITIAL DOCUMENT ===========================
fuzz_test.go:32: r=9999-12-31 23:59:60z
fuzz_test.go:40: DECODED VALUE ===========================
fuzz_test.go:41: map[string]interface {}{"r":time.Date(10000, time.January, 1, 0, 0, 0, 0, time.UTC)}
fuzz_test.go:48: ENCODED DOCUMENT ===========================
fuzz_test.go:49: r = 10000-01-01T00:00:00Z

fuzz_test.go:54: failed round trip: toml: couldn't parse decimal number: strconv.ParseInt: parsing "10000-01-01": invalid syntax
pelletier commented 8 months ago

There seem to be two problems here:

  1. Date time 9999-12-31 23:59:60z is mapped to 10000-01-01 00:00:00z, presumably because Go time.Time does not handle leap seconds and gets normalized by time.Date.
  2. The parser assumes at most four digits in a year. Because of the five-digit year, it attempts to parse the value as an integer instead.

Problem 2 can be solved by being smarter, although I'm worried about the potential performance impact of attempting to parse integers as dates and backtrack more often.

Problem 1: I don't know what to do. Should this be an error saying leap seconds are not supported? Should this be an exception in the round-tripping rule?

arp242 commented 7 months ago

Here's what other parsers do:

% ./run test <<<'d=9999-12-31 23:59:60'
c++-toml++                    ERROR
c++-toml11                    {"d":{"type":"datetime-local","value":"9999-12-31T23:59:60"}}
c-tomlc99                     {"d":{"type":"datetime-local","value":"9999-12-31T23:59:60"}}
cs-tomlyn                     ERROR
dart-toml.dart                {"d":{"type":"datetime-local","value":"9999-12-3123:59:60"}}
fortran-toml-f                {"d":{"type":"datetime","value":"9999-12-3123:59:60"}}
go-go-toml                    {"d":{"type":"datetime-local","value":"9999-12-31T23:59:60"}}
go-toml                       ERROR
js-iarna-toml                 ERROR
js-j-toml                     ERROR
js-smol-toml                  ERROR
js-toml-eslint-parser         {"d":{"type":"datetime-local","value":"9999-12-31T23:59:59.000"}}
lisp-clop                     ERROR
python-toml                   ERROR
python-tomli                  ERROR
python-tomlkit                ERROR
python-tomllib                ERROR
racket-toml-racket            ERROR
ruby-perfect_toml             {"d":{"type":"datetime-local","value":"9999-12-31T23:59:60"}}
ruby-toml-rb                  {"d":{"type":"datetime","value":"10000-01-01T00:00:00Z"}}
ruby-tomlrb                   {"d":{"type":"datetime-local","value":"10000-01-01T00:00:00"}}
rust-basic-toml               ERROR
rust-taplo                    ERROR
rust-toml                     {"d":{"type":"datetime-local","value":"9999-12-31T23:59:60"}}
rust-toml_edit                {"d":{"type":"datetime-local","value":"9999-12-31T23:59:60"}}
% ./run test <<<'d=9999-12-31 23:59:60z'
c++-toml++                    ERROR
c++-toml11                    {"d":{"type":"datetime","value":"9999-12-31T23:59:60Z"}}
c-tomlc99                     {"d":{"type":"datetime","value":"9999-12-31T23:59:60Z"}}
cs-tomlyn                     ERROR
dart-toml.dart                {"d":{"type":"datetime","value":"9999-12-3123:59:60Z"}}
fortran-toml-f                {"d":{"type":"datetime","value":"9999-12-3123:59:60z"}}
go-go-toml                    {"d":{"type":"datetime","value":"10000-01-01T00:00:00Z"}}
go-toml                       ERROR
js-iarna-toml                 ERROR
js-j-toml                     {"d":{"type":"datetime","value":"9999-12-31T23:59:59Z"}}
js-smol-toml                  ERROR
js-toml-eslint-parser         {"d":{"type":"datetime","value":"9999-12-31T23:59:59.000Z"}}
lisp-clop                     ERROR
python-toml                   ERROR
python-tomli                  ERROR
python-tomlkit                ERROR
python-tomllib                ERROR
racket-toml-racket            ERROR
ruby-perfect_toml             {"d":{"type":"datetime","value":"10000-01-01T00:00:00Z"}}
ruby-toml-rb                  ERROR
ruby-tomlrb                   {"d":{"type":"datetime","value":"10000-01-01T00:00:00Z"}}
rust-basic-toml               ERROR
rust-taplo                    ERROR
rust-toml                     {"d":{"type":"datetime","value":"9999-12-31T23:59:60Z"}}
rust-toml_edit                {"d":{"type":"datetime","value":"9999-12-31T23:59:60Z"}}

It's pretty inconsistent.

Leap seconds are going away in a few years anyway; I'm not sure if it's worth excessively worrying about this. I considered adding a test to toml-test for this, but I don't think it's worth it.


Note that 10000-01-01 00:00:00z is not a valid date; year 9999 is the maximum value in RFC3339. The error isn't ideal, but quite a few parsers seem to give suboptimal errors.

pelletier commented 7 months ago

Thanks a lot for the analysis! I'm opting to emit an error when trying to parse a leap second. This seems less surprising than automatically mapping to the second after or before to make the time package happy.