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

time: ParseDuration can panic on invalid input #46883

Closed
athomason opened this issue Jun 23, 2021 · 4 comments
Closed

time: ParseDuration can panic on invalid input #46883

athomason opened this issue Jun 23, 2021 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@athomason
Copy link

What version of Go are you using (go version)?

$ go1.17beta1 version
go version go1.17beta1 darwin/amd64
$ gotip version
go version devel go1.17-ff6f2051d9 Tue Jun 22 04:10:24 2021 +0000 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

darwin/amd64

What did you do?

While experimenting with the awesome new fuzzer, I encountered a class of panics in time.ParseDuration when it receives invalid input and calls into time.quote() to format an error message (playground):

package main

import (
	"fmt"
	"time"
)

func main() {
	if _, err := time.ParseDuration("\x85\x85"); err != nil {
		fmt.Println(err)
	}
}
$ gotip run parse_duration_panic.go
panic: runtime error: index out of range [2] with length 2

goroutine 1 [running]:
time.quote({0x10a209b, 0x2})
    /Users/adam/sdk/gotip/src/time/format.go:770 +0x45f
time.ParseDuration({0x10a209b, 0x2})
    /Users/adam/sdk/gotip/src/time/format.go:1500 +0x393
main.main()
    /Users/adam/parse_duration_panic.go:9 +0x25
exit status 2

time/format.go:770 is in time.quote, which was refactored in https://go-review.googlesource.com/c/go/+/267017, though I haven't bisected to be sure that's the cause.

What did you expect to see?

$ go1.16.5 run parse_duration_panic.go
time: invalid duration "��"

What did you see instead?

$ go1.17beta1 run parse_duration_panic.go
panic: runtime error: index out of range [2] with length 2

goroutine 1 [running]:
time.quote({0x10a209b, 0x2})
    /Users/adam/sdk/go1.17beta1/src/time/format.go:770 +0x45f
time.ParseDuration({0x10a209b, 0x2})
    /Users/adam/sdk/go1.17beta1/src/time/format.go:1500 +0x393
main.main()
    /Users/adam/parse_duration_panic.go:9 +0x25
exit status 2
$ gotip run parse_duration_panic.go
panic: runtime error: index out of range [2] with length 2

goroutine 1 [running]:
time.quote({0x10a209b, 0x2})
    /Users/adam/sdk/gotip/src/time/format.go:770 +0x45f
time.ParseDuration({0x10a209b, 0x2})
    /Users/adam/sdk/gotip/src/time/format.go:1500 +0x393
main.main()
    /Users/adam/parse_duration_panic.go:9 +0x25
exit status 2
@gopherbot
Copy link

Change https://golang.org/cl/330289 mentions this issue: time: capture the non-ASCII rune(65533) in quote() to prevent panic

@kevinburke
Copy link
Contributor

Thank you very much for running a fuzzer and coming up with a reduced test case (something I should have done myself).

@athomason
Copy link
Author

Thanks for the quick turnaround! FWIW I cherrypicked CL 330289 onto dev.fuzz and haven't found any more issues in ~10hr of fuzzing.

@seankhliao seankhliao added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 23, 2021
@odeke-em odeke-em added this to the Go1.17 milestone Jun 24, 2021
@martisch
Copy link
Contributor

static detection for this could be attempted. I had filled an issue for this as dominikh/go-tools#751 when using utf8.RuneLen. Seems len(string(rune)) is a similar problem case.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants