From 1823ae8bc41e1ee82e6796747faf7ee3e40fc855 Mon Sep 17 00:00:00 2001 From: Bplotka Date: Thu, 16 Mar 2017 14:16:20 +0000 Subject: [PATCH] Fixed int64 overflow for timestamp in v1/api parseDuration and parseTime (#2501) * Fixed int64 overflow for timestamp in v1/api parseDuration and parseTime This led to unexpected results on wrong query with "(...)&start=148966367200.372&end=1489667272.372" That query is wrong because of `start > end` but actually internal int64 overflow caused start to be something around MinInt64 (huge negative value) and was passing validation. BTW: Not sure if negative timestamp makes sense even.. But model.Earliest is actually MinInt64, can someone explain me why? Signed-off-by: Bartek Plotka * Added missing trailing periods on comments. Signed-off-by: Bartek Plotka * MOved to only `<` and `>`. Removed equal. Signed-off-by: Bartek Plotka --- web/api/v1/api.go | 14 +++++++++++--- web/api/v1/api_test.go | 34 +++++++++++++++++++++++++++++----- 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/web/api/v1/api.go b/web/api/v1/api.go index b7e9512f8..4f3e25f78 100644 --- a/web/api/v1/api.go +++ b/web/api/v1/api.go @@ -17,6 +17,7 @@ import ( "encoding/json" "errors" "fmt" + "math" "net/http" "sort" "strconv" @@ -477,8 +478,11 @@ func respondError(w http.ResponseWriter, apiErr *apiError, data interface{}) { func parseTime(s string) (model.Time, error) { if t, err := strconv.ParseFloat(s, 64); err == nil { - ts := int64(t * float64(time.Second)) - return model.TimeFromUnixNano(ts), nil + ts := t * float64(time.Second) + if ts > float64(math.MaxInt64) || ts < float64(math.MinInt64) { + return 0, fmt.Errorf("cannot parse %q to a valid timestamp. It overflows int64", s) + } + return model.TimeFromUnixNano(int64(ts)), nil } if t, err := time.Parse(time.RFC3339Nano, s); err == nil { return model.TimeFromUnixNano(t.UnixNano()), nil @@ -488,7 +492,11 @@ func parseTime(s string) (model.Time, error) { func parseDuration(s string) (time.Duration, error) { if d, err := strconv.ParseFloat(s, 64); err == nil { - return time.Duration(d * float64(time.Second)), nil + ts := d * float64(time.Second) + if ts > float64(math.MaxInt64) || ts < float64(math.MinInt64) { + return 0, fmt.Errorf("cannot parse %q to a valid duration. It overflows int64", s) + } + return time.Duration(ts), nil } if d, err := model.ParseDuration(s); err == nil { return time.Duration(d), nil diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index 07f371069..3a26c83ac 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -221,7 +221,7 @@ func TestEndpoints(t *testing.T) { }, errType: errorBadData, }, - // Invalid step + // Invalid step. { endpoint: api.queryRange, query: url.Values{ @@ -232,7 +232,7 @@ func TestEndpoints(t *testing.T) { }, errType: errorBadData, }, - // Start after end + // Start after end. { endpoint: api.queryRange, query: url.Values{ @@ -243,6 +243,17 @@ func TestEndpoints(t *testing.T) { }, errType: errorBadData, }, + // Start overflows int64 internally. + { + endpoint: api.queryRange, + query: url.Values{ + "query": []string{"time()"}, + "start": []string{"148966367200.372"}, + "end": []string{"1489667272.372"}, + "step": []string{"1"}, + }, + errType: errorBadData, + }, { endpoint: api.labelValues, params: map[string]string{ @@ -593,15 +604,20 @@ func TestParseTime(t *testing.T) { }, { input: "30s", fail: true, + }, { + // Internal int64 overflow. + input: "-148966367200.372", + fail: true, + }, { + // Internal int64 overflow. + input: "148966367200.372", + fail: true, }, { input: "123", result: time.Unix(123, 0), }, { input: "123.123", result: time.Unix(123, 123000000), - }, { - input: "123.123", - result: time.Unix(123, 123000000), }, { input: "2015-06-03T13:21:58.555Z", result: ts, @@ -643,6 +659,14 @@ func TestParseDuration(t *testing.T) { }, { input: "2015-06-03T13:21:58.555Z", fail: true, + }, { + // Internal int64 overflow. + input: "-148966367200.372", + fail: true, + }, { + // Internal int64 overflow. + input: "148966367200.372", + fail: true, }, { input: "123", result: 123 * time.Second,