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

OSS-Fuzz issue 63789 #913

Open
oss-fuzz-robot opened this issue Nov 2, 2023 · 4 comments
Open

OSS-Fuzz issue 63789 #913

oss-fuzz-robot opened this issue Nov 2, 2023 · 4 comments
Labels
bug Issues describing a bug in go-toml. fuzzy Issue reported by fuzzy testing, or related to the fuzzing implementation.

Comments

@oss-fuzz-robot
Copy link

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
Copy link
Owner

pelletier commented Nov 2, 2023

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 pelletier added bug Issues describing a bug in go-toml. fuzzy Issue reported by fuzzy testing, or related to the fuzzing implementation. labels Nov 2, 2023
@pelletier
Copy link
Owner

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
Copy link
Contributor

arp242 commented Dec 3, 2023

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.

arp242 added a commit to toml-lang/toml-test that referenced this issue Dec 3, 2023
@pelletier
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues describing a bug in go-toml. fuzzy Issue reported by fuzzy testing, or related to the fuzzing implementation.
Projects
None yet
Development

No branches or pull requests

3 participants