Use single shared random string generation function (#15741)

* Use single shared random string generation function - Replace 3 functions that do the same with 1 shared one - Use crypto/rand over math/rand for a stronger RNG - Output only alphanumerical for URL compatibilty Fixes: #15536 * use const string method * Update modules/avatar/avatar.go Co-authored-by: a1012112796 <[email protected]> Co-authored-by: a1012112796 <[email protected]>
go-gitea · May 10, 2021 · 1e6fa57 · 1e6fa57
1 parent 270aab4
commit 1e6fa57
Show file tree

Hide file tree

Showing 15 changed files with 99 additions and 191 deletions.
diff --git a/models/migrations/v71.go b/models/migrations/v71.go
@@ -8,8 +8,8 @@ import (
 	"crypto/sha256"
 	"fmt"
 
-	"code.gitea.io/gitea/modules/generate"
 	"code.gitea.io/gitea/modules/timeutil"
+	"code.gitea.io/gitea/modules/util"
 
 	"golang.org/x/crypto/pbkdf2"
 	"xorm.io/xorm"
@@ -53,7 +53,7 @@ func addScratchHash(x *xorm.Engine) error {
 
 		for _, tfa := range tfas {
 			// generate salt
-			salt, err := generate.GetRandomString(10)
+			salt, err := util.RandomString(10)
 			if err != nil {
 				return err
 			}

diff --git a/models/migrations/v85.go b/models/migrations/v85.go
@@ -7,9 +7,9 @@ package migrations
 import (
 	"fmt"
 
-	"code.gitea.io/gitea/modules/generate"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/timeutil"
+	"code.gitea.io/gitea/modules/util"
 
 	"xorm.io/xorm"
 )
@@ -65,7 +65,7 @@ func hashAppToken(x *xorm.Engine) error {
 
 		for _, token := range tokens {
 			// generate salt
-			salt, err := generate.GetRandomString(10)
+			salt, err := util.RandomString(10)
 			if err != nil {
 				return err
 			}

diff --git a/models/token.go b/models/token.go
@@ -10,8 +10,8 @@ import (
 	"time"
 
 	"code.gitea.io/gitea/modules/base"
-	"code.gitea.io/gitea/modules/generate"
 	"code.gitea.io/gitea/modules/timeutil"
+	"code.gitea.io/gitea/modules/util"
 
 	gouuid "github.com/google/uuid"
 )
@@ -40,7 +40,7 @@ func (t *AccessToken) AfterLoad() {
 
 // NewAccessToken creates new access token.
 func NewAccessToken(t *AccessToken) error {
-	salt, err := generate.GetRandomString(10)
+	salt, err := util.RandomString(10)
 	if err != nil {
 		return err
 	}

diff --git a/models/twofactor.go b/models/twofactor.go
@@ -11,10 +11,10 @@ import (
 	"encoding/base64"
 	"fmt"
 
-	"code.gitea.io/gitea/modules/generate"
 	"code.gitea.io/gitea/modules/secret"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/timeutil"
+	"code.gitea.io/gitea/modules/util"
 
 	"github.com/pquerna/otp/totp"
 	"golang.org/x/crypto/pbkdf2"
@@ -34,11 +34,11 @@ type TwoFactor struct {
 
 // GenerateScratchToken recreates the scratch token the user is using.
 func (t *TwoFactor) GenerateScratchToken() (string, error) {
-	token, err := generate.GetRandomString(8)
+	token, err := util.RandomString(8)
 	if err != nil {
 		return "", err
 	}
-	t.ScratchSalt, _ = generate.GetRandomString(10)
+	t.ScratchSalt, _ = util.RandomString(10)
 	t.ScratchHash = hashToken(token, t.ScratchSalt)
 	return token, nil
 }

diff --git a/models/user.go b/models/user.go
@@ -22,7 +22,6 @@ import (
 	"unicode/utf8"
 
 	"code.gitea.io/gitea/modules/base"
-	"code.gitea.io/gitea/modules/generate"
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
@@ -746,7 +745,7 @@ func IsUserExist(uid int64, name string) (bool, error) {
 
 // GetUserSalt returns a random user salt token.
 func GetUserSalt() (string, error) {
-	return generate.GetRandomString(10)
+	return util.RandomString(10)
 }
 
 // NewGhostUser creates and returns a fake user for someone has deleted his/her account.

diff --git a/modules/avatar/avatar.go b/modules/avatar/avatar.go
@@ -12,10 +12,9 @@ import (
 
 	// Enable PNG support:
 	_ "image/png"
-	"math/rand"
-	"time"
 
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/util"
 
 	"github.com/issue9/identicon"
 	"github.com/nfnt/resize"
@@ -29,8 +28,11 @@ const AvatarSize = 290
 // in custom size (height and width).
 func RandomImageSize(size int, data []byte) (image.Image, error) {
 	randExtent := len(palette.WebSafe) - 32
-	rand.Seed(time.Now().UnixNano())
-	colorIndex := rand.Intn(randExtent)
+	integer, err := util.RandomInt(int64(randExtent))
+	if err != nil {
+		return nil, fmt.Errorf("util.RandomInt: %v", err)
+	}
+	colorIndex := int(integer)
 	backColorIndex := colorIndex - 1
 	if backColorIndex < 0 {
 		backColorIndex = randExtent - 1

diff --git a/modules/avatar/avatar_test.go b/modules/avatar/avatar_test.go
@@ -13,12 +13,17 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func Test_RandomImage(t *testing.T) {
-	_, err := RandomImage([]byte("gogs@local"))
+func Test_RandomImageSize(t *testing.T) {
+	_, err := RandomImageSize(0, []byte("gitea@local"))
+	assert.Error(t, err)
+
+	_, err = RandomImageSize(64, []byte("gitea@local"))
 	assert.NoError(t, err)
+}
 
-	_, err = RandomImageSize(0, []byte("gogs@local"))
-	assert.Error(t, err)
+func Test_RandomImage(t *testing.T) {
+	_, err := RandomImage([]byte("gitea@local"))
+	assert.NoError(t, err)
 }
 
 func Test_PrepareWithPNG(t *testing.T) {

diff --git a/modules/context/secret.go b/modules/context/secret.go
diff --git a/modules/generate/generate.go b/modules/generate/generate.go
@@ -9,31 +9,12 @@ import (
 	"crypto/rand"
 	"encoding/base64"
 	"io"
-	"math/big"
 	"time"
 
+	"code.gitea.io/gitea/modules/util"
 	"github.com/dgrijalva/jwt-go"
 )
 
-// GetRandomString generate random string by specify chars.
-func GetRandomString(n int) (string, error) {
-	const alphanum = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
-
-	buffer := make([]byte, n)
-	max := big.NewInt(int64(len(alphanum)))
-
-	for i := 0; i < n; i++ {
-		index, err := randomInt(max)
-		if err != nil {
-			return "", err
-		}
-
-		buffer[i] = alphanum[index]
-	}
-
-	return string(buffer), nil
-}
-
 // NewInternalToken generate a new value intended to be used by INTERNAL_TOKEN.
 func NewInternalToken() (string, error) {
 	secretBytes := make([]byte, 32)
@@ -69,19 +50,10 @@ func NewJwtSecret() (string, error) {
 
 // NewSecretKey generate a new value intended to be used by SECRET_KEY.
 func NewSecretKey() (string, error) {
-	secretKey, err := GetRandomString(64)
+	secretKey, err := util.RandomString(64)
 	if err != nil {
 		return "", err
 	}
 
 	return secretKey, nil
 }
-
-func randomInt(max *big.Int) (int, error) {
-	rand, err := rand.Int(rand.Reader, max)
-	if err != nil {
-		return 0, err
-	}
-
-	return int(rand.Int64()), nil
-}
diff --git a/modules/generate/generate_test.go b/modules/generate/generate_test.go
diff --git a/modules/secret/secret.go b/modules/secret/secret.go
@@ -13,29 +13,18 @@ import (
 	"encoding/hex"
 	"errors"
 	"io"
+
+	"code.gitea.io/gitea/modules/util"
 )
 
 // New creats a new secret
 func New() (string, error) {
-	return NewWithLength(32)
+	return NewWithLength(44)
 }
 
 // NewWithLength creates a new secret for a given length
 func NewWithLength(length int64) (string, error) {
-	return randomString(length)
-}
-
-func randomBytes(len int64) ([]byte, error) {
-	b := make([]byte, len)
-	if _, err := rand.Read(b); err != nil {
-		return nil, err
-	}
-	return b, nil
-}
-
-func randomString(len int64) (string, error) {
-	b, err := randomBytes(len)
-	return base64.URLEncoding.EncodeToString(b), err
+	return util.RandomString(length)
 }
 
 // AesEncrypt encrypts text and given key with AES.

diff --git a/modules/secret/secret_test.go b/modules/secret/secret_test.go
@@ -13,7 +13,7 @@ import (
 func TestNew(t *testing.T) {
 	result, err := New()
 	assert.NoError(t, err)
-	assert.True(t, len(result) > 32)
+	assert.True(t, len(result) == 44)
 
 	result2, err := New()
 	assert.NoError(t, err)