Fixed int64 overflow for timestamp in v1/api parseDuration and parseT…

…ime (#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 <bwplotka@gmail.com> * Added missing trailing periods on comments. Signed-off-by: Bartek Plotka <bwplotka@gmail.com> * MOved to only `<` and `>`. Removed equal. Signed-off-by: Bartek Plotka <bwplotka@gmail.com>
prometheus · Mar 16, 2017 · 1823ae8 · 1823ae8
1 parent 48d221c
commit 1823ae8
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 8 deletions.
diff --git 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
@@ -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,