Repository: prometheus/common
Pull request #285 (2021-03-17 17:30:47)

Fix #8526 (https://github.com/prometheus/prometheus/issues/8526)
Fix #8526 (https://github.com/prometheus/prometheus/issues/8526) issue by validating if the input range exceeds 290 years on "func ParseDuration(durationStr string) (Duration, error)".

The problem reported on this issue occurs because the "time.Duration" on Go is limited to 290 years. When you exceed this limit you get unexpected behaviours.

Fixes prometheus/prometheus#8526

wfelipew: > Thanks!!
> 
> I think we could detect also more complex overflows like 200y10400w. What do you think of this kind of approach:
> 
> ```diff
> diff --git a/model/time.go b/model/time.go
> index e684ebd..3973d6a 100644
> --- a/model/time.go
> +++ b/model/time.go
> @@ -15,6 +15,7 @@ package model
>  
>  import (
>  	"encoding/json"
> +	"errors"
>  	"fmt"
>  	"math"
>  	"regexp"
> @@ -202,13 +203,20 @@ func ParseDuration(durationStr string) (Duration, error) {
>  
>  	// Parse the match at pos `pos` in the regex and use `mult` to turn that
>  	// into ms, then add that value to the total parsed duration.
> +	var overflowErr error
>  	m := func(pos int, mult time.Duration) {
>  		if matches[pos] == "" {
>  			return
>  		}
>  		n, _ := strconv.Atoi(matches[pos])
> +		if n > int((1<<63-1)/mult/time.Millisecond) {
> +			overflowErr = errors.New("overflow")
> +		}
>  		d := time.Duration(n) * time.Millisecond
>  		dur += d * mult
> +		if dur < 0 {
> +			overflowErr = errors.New("overflow")
> +		}
>  	}
>  
>  	m(2, 1000*60*60*24*365) // y
> @@ -219,7 +227,7 @@ func ParseDuration(durationStr string) (Duration, error) {
>  	m(12, 1000)             // s
>  	m(14, 1)                // ms
>  
> -	return Duration(dur), nil
> +	return Duration(dur), overflowErr
>  }
>  
>  func (d Duration) String() string {
> ```

It's fine, I just also add some tests cases
roidelapluie: Thank you. If you rebase on top of main, you will see that we got rid of tests on older versions.

We are very close to merge now 👍🏻 
wfelipew: > Thank you. If you rebase on top of main, you will see that we got rid of tests on older versions.
> 
> We are very close to merge now 👍🏻

Done

Patch:
diff --git a/model/time.go b/model/time.go
index e684ebd..7f67b16 100644
--- a/model/time.go
+++ b/model/time.go
@@ -15,6 +15,7 @@ package model
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"math"
 	"regexp"
@@ -202,13 +203,23 @@ func ParseDuration(durationStr string) (Duration, error) {
 
 	// Parse the match at pos `pos` in the regex and use `mult` to turn that
 	// into ms, then add that value to the total parsed duration.
+	var overflowErr error
 	m := func(pos int, mult time.Duration) {
 		if matches[pos] == "" {
 			return
 		}
 		n, _ := strconv.Atoi(matches[pos])
+
+		// Check if the provided duration overflows time.Duration (> ~ 290years).
+		if n > int((1<<63-1)/mult/time.Millisecond) {
+			overflowErr = errors.New("duration out of range")
+		}
 		d := time.Duration(n) * time.Millisecond
 		dur += d * mult
+
+		if dur < 0 {
+			overflowErr = errors.New("duration out of range")
+		}
 	}
 
 	m(2, 1000*60*60*24*365) // y
@@ -219,7 +230,7 @@ func ParseDuration(durationStr string) (Duration, error) {
 	m(12, 1000)             // s
 	m(14, 1)                // ms
 
-	return Duration(dur), nil
+	return Duration(dur), overflowErr
 }
 
 func (d Duration) String() string {


Test patch:
diff --git a/model/time_test.go b/model/time_test.go
index a65b859..5ebebd5 100644
--- a/model/time_test.go
+++ b/model/time_test.go
@@ -282,6 +282,10 @@ func TestDuration_UnmarshalJSON(t *testing.T) {
 			in:  `"10y"`,
 			out: 10 * 365 * 24 * time.Hour,
 		},
+		{
+			in:  `"289y"`,
+			out: 289 * 365 * 24 * time.Hour,
+		},
 	}
 
 	for _, c := range cases {
@@ -314,6 +318,10 @@ func TestParseBadDuration(t *testing.T) {
 		"-1w",
 		"1.5d",
 		"d",
+		"294y",
+		"200y10400w",
+		"107675d",
+		"2584200h",
 		"",
 	}