CVE-2021-4238: Insufficient Entropy (CWE-331)
value: Randomly-generated alphanumeric strings contain significantly less entropy than expected. The RandomAlphaNumeric and CryptoRandomAlphaNumeric functions always return strings containing at least one digit from 0 to 9. This significantly reduces the amount of entropy in short strings generated by these functions.
CWE description: The product uses an algorithm or scheme that produces insufficient entropy, leaving patterns or clusters of values that are more likely to occur than others.
CVSS v3 base score: 9.1
Published: 2022-12-27T22:15Z
Repository: https://github.com/Masterminds/goutils
Language: Go
Files changed: cryptorandomstringutils.go, cryptorandomstringutils_test.go, randomstringutils.go, randomstringutils_test.go
Commit date: 2021-01-28 14:00:48 -0700
Commit message: Remove unnecessary checks on a value that is already definitely an alphanum

Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>

Diff: diff --git a/cryptorandomstringutils.go b/cryptorandomstringutils.go
index 177dd86..b366073 100644
--- a/cryptorandomstringutils.go
+++ b/cryptorandomstringutils.go
@@ -19,11 +19,10 @@ package goutils
 import (
 	"crypto/rand"
 	"fmt"
 	"math"
 	"math/big"
-	"regexp"
 	"unicode"
 )
 
 /*
 CryptoRandomNonAlphaNumeric creates a random string whose length is the number of characters specified.
@@ -100,28 +99,12 @@ Returns:
 */
 func CryptoRandomAlphaNumeric(count int) (string, error) {
 	if count == 0 {
 		return "", nil
 	}
-	RandomString, err := CryptoRandom(count, 0, 0, true, true)
-	if err != nil {
-		return "", fmt.Errorf("Error: %s", err)
-	}
-	match, err := regexp.MatchString("([0-9]+)", RandomString)
-	if err != nil {
-		panic(err)
-	}
-
-	if !match {
-		//Get the position between 0 and the length of the string-1  to insert a random number
-		position := getCryptoRandomInt(count)
-		//Insert a random number between [0-9] in the position
-		RandomString = RandomString[:position] + string('0' + getCryptoRandomInt(10)) + RandomString[position + 1:]
-		return RandomString, err
-	}
-	return RandomString, err
 
+	return CryptoRandom(count, 0, 0, true, true)
 }
 
 /*
 CryptoRandomAlphaNumericCustom creates a random string whose length is the number of characters specified.
 Characters will be chosen from the set of alpha-numeric characters as indicated by the arguments.
@@ -202,11 +185,11 @@ func CryptoRandom(count int, start int, end int, letters bool, numbers bool, cha
 		count--
 		var ch rune
 		if chars == nil {
 			ch = rune(getCryptoRandomInt(gap) + int64(start))
 		} else {
-			ch = chars[getCryptoRandomInt(gap) + int64(start)]
+			ch = chars[getCryptoRandomInt(gap)+int64(start)]
 		}
 
 		if letters && unicode.IsLetter(ch) || numbers && unicode.IsDigit(ch) || !letters && !numbers {
 			if ch >= 56320 && ch <= 57343 { // low surrogate range
 				if count == 0 {
diff --git a/cryptorandomstringutils_test.go b/cryptorandomstringutils_test.go
index c8c1622..eb034df 100644
--- a/cryptorandomstringutils_test.go
+++ b/cryptorandomstringutils_test.go
@@ -1,8 +1,10 @@
 package goutils
 
 import (
+	"regexp"
+	"strconv"
 	"testing"
 	"unicode/utf8"
 )
 
 func TestCryptoRandomNonAlphaNumeric(t *testing.T) {
@@ -72,5 +74,40 @@ func TestCryptoRandomAlphaNumeric(t *testing.T) {
 		if x, _ := CryptoRandomAlphaNumeric(i); len(x) != i {
 			t.Errorf("String should be %v characters; string was %v characters", i, len(x))
 		}
 	}
 }
+
+func TestCryptoRandAlphaNumeric_FuzzOnlyNumeric(t *testing.T) {
+
+	// Testing for a reported regression in which some versions produced
+	// a predictably small set of chars.
+	iters := 1000
+	charlen := 0
+	for i := 0; i < 16; i++ {
+		numOnly := 0
+		charlen++
+		for i := 0; i < iters; i++ {
+			out, err := CryptoRandomAlphaNumeric(charlen)
+			println(out)
+			if err != nil {
+				t.Fatal("func failed to produce a random thinger")
+			}
+			if _, err := strconv.Atoi(out); err == nil {
+				numOnly++
+			}
+
+			m, err := regexp.MatchString("^[0-9a-zA-Z]+$", out)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if !m {
+				t.Fatal("Character is not alphanum")
+			}
+		}
+
+		if numOnly == iters {
+			t.Fatalf("Got %d numeric-only random sequences", numOnly)
+		}
+	}
+
+}
diff --git a/randomstringutils.go b/randomstringutils.go
index 1364e0c..8c6ccab 100644
--- a/randomstringutils.go
+++ b/randomstringutils.go
@@ -18,11 +18,10 @@ package goutils
 
 import (
 	"fmt"
 	"math"
 	"math/rand"
-	"regexp"
 	"time"
 	"unicode"
 )
 
 // RANDOM provides the time-based seed used to generate random numbers
@@ -73,16 +72,14 @@ func RandomNumeric(count int) (string, error) {
 	return Random(count, 0, 0, false, true)
 }
 
 /*
 RandomAlphabetic creates a random string whose length is the number of characters specified.
-Characters will be chosen from the set of alpha-numeric characters as indicated by the arguments.
+Characters will be chosen from the set of alphabetic characters.
 
 Parameters:
 	count - the length of random string to create
-	letters - if true, generated string may include alphabetic characters
-	numbers - if true, generated string may include numeric characters
 
 Returns:
 	string - the random string
 	error - an error stemming from an invalid parameter within underlying function, RandomSeed(...)
 */
@@ -104,23 +101,12 @@ Returns:
 func RandomAlphaNumeric(count int) (string, error) {
 	RandomString, err := Random(count, 0, 0, true, true)
 	if err != nil {
 		return "", fmt.Errorf("Error: %s", err)
 	}
-	match, err := regexp.MatchString("([0-9]+)", RandomString)
-	if err != nil {
-		panic(err)
-	}
 
-	if !match {
-		//Get the position between 0 and the length of the string-1  to insert a random number
-		position := rand.Intn(count)
-		//Insert a random number between [0-9] in the position
-		RandomString = RandomString[:position] + string('0'+rand.Intn(10)) + RandomString[position+1:]
-		return RandomString, err
-	}
-	return RandomString, err
+	return RandomString[:count], nil
 
 }
 
 /*
 RandomAlphaNumericCustom creates a random string whose length is the number of characters specified.
diff --git a/randomstringutils_test.go b/randomstringutils_test.go
index b990654..9018e43 100644
--- a/randomstringutils_test.go
+++ b/randomstringutils_test.go
@@ -1,10 +1,12 @@
 package goutils
 
 import (
 	"fmt"
 	"math/rand"
+	"regexp"
+	"strconv"
 	"testing"
 )
 
 // ****************************** TESTS ********************************************
 
@@ -74,5 +76,40 @@ func ExampleRandomSeed() {
 	// MBrbj
 	// 88935
 	// H_I;E
 	// 2b2ca
 }
+
+func TestRandAlphaNumeric_FuzzOnlyNumeric(t *testing.T) {
+
+	// Testing for a reported regression in which some versions produced
+	// a predictably small set of chars.
+	iters := 1000
+	charlen := 0
+	for i := 0; i < 16; i++ {
+		numOnly := 0
+		charlen++
+		for i := 0; i < iters; i++ {
+			out, err := RandomAlphaNumeric(charlen)
+			println(out)
+			if err != nil {
+				t.Fatal("func failed to produce a random thinger")
+			}
+			if _, err := strconv.Atoi(out); err == nil {
+				numOnly++
+			}
+
+			m, err := regexp.MatchString("^[0-9a-zA-Z]+$", out)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if !m {
+				t.Fatal("Character is not alphanum")
+			}
+		}
+
+		if numOnly == iters {
+			t.Fatalf("Got %d numeric-only random sequences", numOnly)
+		}
+	}
+
+}

Vulnerable code: 	"regexp"
	RandomString, err := CryptoRandom(count, 0, 0, true, true)
	if err != nil {
		return "", fmt.Errorf("Error: %s", err)
	}
	match, err := regexp.MatchString("([0-9]+)", RandomString)
	if err != nil {
		panic(err)
	}

	if !match {
		//Get the position between 0 and the length of the string-1  to insert a random number
		position := getCryptoRandomInt(count)
		//Insert a random number between [0-9] in the position
		RandomString = RandomString[:position] + string('0' + getCryptoRandomInt(10)) + RandomString[position + 1:]
		return RandomString, err
	}
	return RandomString, err
			ch = chars[getCryptoRandomInt(gap) + int64(start)]
	"regexp"
Characters will be chosen from the set of alpha-numeric characters as indicated by the arguments.
	letters - if true, generated string may include alphabetic characters
	numbers - if true, generated string may include numeric characters
	match, err := regexp.MatchString("([0-9]+)", RandomString)
	if err != nil {
		panic(err)
	}
	if !match {
		//Get the position between 0 and the length of the string-1  to insert a random number
		position := rand.Intn(count)
		//Insert a random number between [0-9] in the position
		RandomString = RandomString[:position] + string('0'+rand.Intn(10)) + RandomString[position+1:]
		return RandomString, err
	}
	return RandomString, err
Fixed code: 	return CryptoRandom(count, 0, 0, true, true)
			ch = chars[getCryptoRandomInt(gap)+int64(start)]
	"regexp"
	"strconv"

func TestCryptoRandAlphaNumeric_FuzzOnlyNumeric(t *testing.T) {

	// Testing for a reported regression in which some versions produced
	// a predictably small set of chars.
	iters := 1000
	charlen := 0
	for i := 0; i < 16; i++ {
		numOnly := 0
		charlen++
		for i := 0; i < iters; i++ {
			out, err := CryptoRandomAlphaNumeric(charlen)
			println(out)
			if err != nil {
				t.Fatal("func failed to produce a random thinger")
			}
			if _, err := strconv.Atoi(out); err == nil {
				numOnly++
			}

			m, err := regexp.MatchString("^[0-9a-zA-Z]+$", out)
			if err != nil {
				t.Fatal(err)
			}
			if !m {
				t.Fatal("Character is not alphanum")
			}
		}

		if numOnly == iters {
			t.Fatalf("Got %d numeric-only random sequences", numOnly)
		}
	}

}
Characters will be chosen from the set of alphabetic characters.
	return RandomString[:count], nil
	"regexp"
	"strconv"

func TestRandAlphaNumeric_FuzzOnlyNumeric(t *testing.T) {

	// Testing for a reported regression in which some versions produced
	// a predictably small set of chars.
	iters := 1000
	charlen := 0
	for i := 0; i < 16; i++ {
		numOnly := 0
		charlen++
		for i := 0; i < iters; i++ {
			out, err := RandomAlphaNumeric(charlen)
			println(out)
			if err != nil {
				t.Fatal("func failed to produce a random thinger")
			}
			if _, err := strconv.Atoi(out); err == nil {
				numOnly++
			}

			m, err := regexp.MatchString("^[0-9a-zA-Z]+$", out)
			if err != nil {
				t.Fatal(err)
			}
			if !m {
				t.Fatal("Character is not alphanum")
			}
		}

		if numOnly == iters {
			t.Fatalf("Got %d numeric-only random sequences", numOnly)
		}
	}

}