From 6a0d55c0f149cf9bea1ce70aa587b521a9c6d055 Mon Sep 17 00:00:00 2001 From: Eric Cornelissen Date: Thu, 31 Dec 2020 22:33:24 +0100 Subject: [PATCH] Fix panic due to invalid schedule string Fix a bug where cron panics if the schedule string ("spec") has a particular format, namely "TZ=" or "CRON_TZ=". If this is string is user controlled that user may be able to panic and thereby exit the program using cron. Alternatively, if the string is constructed dynamically by the program and there is a mistake in that logic, it would panic unexpectedly. The solution I propose is simply to check against the condition that causes cron to panic. There are alternative solution, e.g.: - Using more complex logic (using e.g. regexp) to detect the "TZ"-syntax. - Updating parsing logic so that the "provided bad location" error is returned. I opted away from this as I figured it might be useful to distinguish these two scenarios. Disclose: I found this bug using https://github.com/dvyukov/go-fuzz and did not encounter any other parsing issues. --- parser.go | 3 +++ parser_test.go | 17 +++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/parser.go b/parser.go index 8da6547a..80ae2913 100644 --- a/parser.go +++ b/parser.go @@ -95,6 +95,9 @@ func (p Parser) Parse(spec string) (Schedule, error) { if strings.HasPrefix(spec, "TZ=") || strings.HasPrefix(spec, "CRON_TZ=") { var err error i := strings.Index(spec, " ") + if i == -1 { + return nil, fmt.Errorf("provided no location %s", spec) + } eq := strings.Index(spec, "=") if loc, err = time.LoadLocation(spec[eq+1 : i]); err != nil { return nil, fmt.Errorf("provided bad location %s: %v", spec[eq+1:i], err) diff --git a/parser_test.go b/parser_test.go index 41c8c520..25cd3dc6 100644 --- a/parser_test.go +++ b/parser_test.go @@ -179,6 +179,23 @@ func TestParseSchedule(t *testing.T) { t.Errorf("%s => expected %b, got %b", c.expr, c.expected, actual) } } + + badEntries := []struct { + parser Parser + expr string + }{ + {secondParser, "TZ="}, + {standardParser, "TZ="}, + {secondParser, "CRON_TZ="}, + {standardParser, "CRON_TZ="}, + } + + for _, c := range badEntries { + _, err := c.parser.Parse(c.expr) + if err == nil { + t.Errorf("%s => expected error but got none", c.expr) + } + } } func TestOptionalSecondSchedule(t *testing.T) {