Fix random string generator (#384)

* Remove unused custom-alphabet feature of random string generator

Fix random string generator

Random string generator should return error if it fails to read random data via crypto/rand

* Fixes variable (un)initialization mixed assign
Update test GetRandomString

2 files changed, 26 insertions, 10 deletions

diff --git a/modules/base/tool.go b/modules/base/tool.go
index 1722c88ac8..eb25108869 100644
--- a/modules/base/tool.go
+++ b/modules/base/tool.go
@@ -13,6 +13,7 @@ import (
 	"fmt"
 	"html/template"
 	"math"
+	"math/big"
 	"net/http"
 	"strconv"
 	"strings"
@@ -81,18 +82,31 @@ func BasicAuthEncode(username, password string) string {
 }
 
 // GetRandomString generate random string by specify chars.
-func GetRandomString(n int, alphabets ...byte) string {
+func GetRandomString(n int) (string, error) {
 	const alphanum = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
-	var bytes = make([]byte, n)
-	rand.Read(bytes)
-	for i, b := range bytes {
-		if len(alphabets) == 0 {
-			bytes[i] = alphanum[b%byte(len(alphanum))]
-		} else {
-			bytes[i] = alphabets[b%byte(len(alphabets))]
+
+	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(bytes)
+
+	return string(buffer), 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
 }
 
 // VerifyTimeLimitCode verify time limit code
diff --git a/modules/base/tool_test.go b/modules/base/tool_test.go
index ec839e5e10..2ca70b8b32 100644
--- a/modules/base/tool_test.go
+++ b/modules/base/tool_test.go
@@ -43,7 +43,9 @@ func TestBasicAuthEncode(t *testing.T) {
 }
 
 func TestGetRandomString(t *testing.T) {
-	assert.Len(t, GetRandomString(4), 4)
+	randomString, err := GetRandomString(4)
+	assert.NoError(t, err)
+	assert.Len(t, randomString, 4)
 }
 
 // TODO: Test PBKDF2()