fix(shacrypt): clamp out of range rounds when decoding

algorithm/shacrypt/decoder.go

@@ -1,8 +1,10 @@
 package shacrypt
 
 import (
+	"errors"
 	"fmt"
 	"strconv"
+	"strings"
 
 	"github.com/go-crypt/crypt/algorithm"
 	"github.com/go-crypt/crypt/internal/encoding"
@@ -120,15 +122,18 @@ func decode(variant Variant, parts []string) (digest algorithm.Digest, err error
 		case "rounds":
 			var rounds uint64
 
-			if rounds, err = strconv.ParseUint(param.Value, 10, 32); err != nil {
+			if rounds, err = strconv.ParseUint(param.Value, 10, 64); err != nil && (!errors.Is(err, strconv.ErrRange) || strings.Trim(param.Value, "0123456789") != "") {
 				return nil, fmt.Errorf("%w: option '%s' has invalid value '%s': %v", algorithm.ErrEncodedHashInvalidOptionValue, param.Key, param.Value, err)
 			}
 
-			if rounds < IterationsMin || rounds > IterationsMax {
-				return nil, fmt.Errorf(algorithm.ErrFmtInvalidIntParameter, algorithm.ErrEncodedHashInvalidOptionValue, param.Key, IterationsMin, "", IterationsMax, rounds)
+			switch {
+			case rounds < IterationsMin:
+				decoded.iterations = IterationsMin
+			case rounds > IterationsMax:
+				decoded.iterations = IterationsMax
+			default:
+				decoded.iterations = int(rounds)
 			}
-
-			decoded.iterations = int(rounds)
 		default:
 			return nil, fmt.Errorf("%w: option '%s' with value '%s' is unknown", algorithm.ErrEncodedHashInvalidOptionKey, param.Key, param.Value)
 		}

algorithm/shacrypt/regression_test.go

@@ -7,15 +7,40 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestDecodeRejectsUnusableRounds(t *testing.T) {
+func TestDecodeClampsOutOfRangeRounds(t *testing.T) {
+	testCases := []struct {
+		name     string
+		digest   string
+		expected string
+	}{
+		{"Zero", "$6$rounds=0$saltsalt$keykeykey", "$6$rounds=1000$saltsalt$keykeykey"},
+		{"BelowMinimum", "$6$rounds=100$saltsalt$keykeykey", "$6$rounds=1000$saltsalt$keykeykey"},
+		{"JustBelowMinimum", "$6$rounds=999$saltsalt$keykeykey", "$6$rounds=1000$saltsalt$keykeykey"},
+		{"JustAboveMaximum", "$6$rounds=1000000000$saltsalt$keykeykey", "$6$rounds=999999999$saltsalt$keykeykey"},
+		{"AboveMaximum", "$6$rounds=4294967295$saltsalt$keykeykey", "$6$rounds=999999999$saltsalt$keykeykey"},
+		{"AboveUint32", "$6$rounds=4294967296$saltsalt$keykeykey", "$6$rounds=999999999$saltsalt$keykeykey"},
+		{"AboveUint64", "$6$rounds=99999999999999999999$saltsalt$keykeykey", "$6$rounds=999999999$saltsalt$keykeykey"},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			digest, err := Decode(tc.digest)
+
+			require.NoError(t, err)
+			assert.Equal(t, tc.expected, digest.Encode())
+		})
+	}
+}
+
+func TestDecodeRejectsInvalidRounds(t *testing.T) {
 	testCases := []struct {
 		name   string
 		digest string
 	}{
-		{"Zero", "$6$rounds=0$saltsalt$keykeykey"},
-		{"BelowMinimum", "$6$rounds=100$saltsalt$keykeykey"},
-		{"JustBelowMinimum", "$6$rounds=999$saltsalt$keykeykey"},
-		{"AboveMaximum", "$6$rounds=4294967295$saltsalt$keykeykey"},
+		{"Empty", "$6$rounds=$saltsalt$keykeykey"},
+		{"Negative", "$6$rounds=-1$saltsalt$keykeykey"},
+		{"NotNumeric", "$6$rounds=abc$saltsalt$keykeykey"},
+		{"AboveUint64NotNumericSuffix", "$6$rounds=99999999999999999999x$saltsalt$keykeykey"},
 	}
 
 	for _, tc := range testCases {
@@ -28,6 +53,38 @@ func TestDecodeRejectsUnusableRounds(t *testing.T) {
 	}
 }
 
+func TestDecodeRoundsTooLowSpecVectors(t *testing.T) {
+	testCases := []struct {
+		name     string
+		digest   string
+		expected string
+	}{
+		{
+			"SHA256",
+			"$5$rounds=10$roundstoolow$yfvwcWrQ8l/K0DAWyuPMDNHpIVlTQebY9l/gL972bIC",
+			"$5$rounds=1000$roundstoolow$yfvwcWrQ8l/K0DAWyuPMDNHpIVlTQebY9l/gL972bIC",
+		},
+		{
+			"SHA512",
+			"$6$rounds=10$roundstoolow$kUMsbe306n21p9R.FRkW3IGn.S9NPN0x50YhH1xhLsPuWGsUSklZt58jaTfF4ZEQpyUNGc0dqbpBYYBaHHrsX.",
+			"$6$rounds=1000$roundstoolow$kUMsbe306n21p9R.FRkW3IGn.S9NPN0x50YhH1xhLsPuWGsUSklZt58jaTfF4ZEQpyUNGc0dqbpBYYBaHHrsX.",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			digest, err := Decode(tc.digest)
+			require.NoError(t, err)
+
+			match, err := digest.MatchAdvanced("the minimum number is still observed")
+
+			require.NoError(t, err)
+			assert.True(t, match)
+			assert.Equal(t, tc.expected, digest.Encode())
+		})
+	}
+}
+
 func TestDecodeAcceptsRoundsWithinRange(t *testing.T) {
 	testCases := []struct {
 		name   string

fuzz_test.go

@@ -1,7 +1,6 @@
 package crypt
 
 import (
-	"strings"
 	"testing"
 )
 
@@ -44,52 +43,6 @@ func FuzzNormalize(f *testing.F) {
 	})
 }
 
-// argon2MemoryHardParameter is the argon2 memory parameter carried by several corpus entries. Deriving a key with it
-// allocates 2GiB which is more than a test run should require, so the entries which carry it and also decode are
-// skipped when verifying passwords. FuzzDecode and FuzzNormalize still cover them as neither derives a key.
-const argon2MemoryHardParameter = "m=2097152"
-
-func TestCheckPasswordNeverPanicsOverCorpus(t *testing.T) {
-	for _, encodedDigest := range corpusDecode {
-		t.Run(encodedDigest, func(t *testing.T) {
-			var (
-				valid bool
-				err   error
-			)
-
-			if strings.Contains(encodedDigest, argon2MemoryHardParameter) {
-				if _, decodeErr := Decode(encodedDigest); decodeErr == nil {
-					t.Skip("deriving a key from this digest would allocate 2GiB; it remains covered by FuzzDecode and FuzzNormalize")
-				}
-			}
-
-			if !assertNotPanics(t, func() { valid, err = CheckPassword("password", encodedDigest) }) {
-				return
-			}
-
-			if valid && err != nil {
-				t.Fatalf("CheckPassword reported a match alongside the error %v", err)
-			}
-		})
-	}
-}
-
-func assertNotPanics(t *testing.T, f func()) (ok bool) {
-	t.Helper()
-
-	defer func() {
-		if r := recover(); r != nil {
-			t.Errorf("panic: %v", r)
-
-			ok = false
-		}
-	}()
-
-	f()
-
-	return true
-}
-
 var corpusDecode = []string{
 	// Well formed digests for each supported identifier.
 	"$argon2id$v=19$m=2097152,t=1,p=4$YmxhaGJsYWhibGFoYmxhaA$Vt4rHrEcdEJ+A9FBAOtqE21NX2NDaCyR3xr0PJmg+dU",