Handle base64 decoding correctly to avoid panic (#26483)

Fix the panic if the "base64 secret" is too long.
go-gitea · Aug 14, 2023 · ed1be4c · ed1be4c
1 parent cafce3b
commit ed1be4c
Show file tree

Hide file tree

Showing 8 changed files with 43 additions and 30 deletions.
diff --git a/cmd/generate.go b/cmd/generate.go
@@ -70,12 +70,12 @@ func runGenerateInternalToken(c *cli.Context) error {
 }
 
 func runGenerateLfsJwtSecret(c *cli.Context) error {
-	JWTSecretBase64, err := generate.NewJwtSecretBase64()
+	_, jwtSecretBase64, err := generate.NewJwtSecretBase64()
 	if err != nil {
 		return err
 	}
 
-	fmt.Printf("%s", JWTSecretBase64)
+	fmt.Printf("%s", jwtSecretBase64)
 
 	if isatty.IsTerminal(os.Stdout.Fd()) {
 		fmt.Printf("\n")

diff --git a/modules/generate/generate.go b/modules/generate/generate.go
@@ -49,12 +49,12 @@ func NewJwtSecret() ([]byte, error) {
 }
 
 // NewJwtSecretBase64 generates a new base64 encoded value intended to be used for JWT secrets.
-func NewJwtSecretBase64() (string, error) {
+func NewJwtSecretBase64() ([]byte, string, error) {
 	bytes, err := NewJwtSecret()
 	if err != nil {
-		return "", err
+		return nil, "", err
 	}
-	return base64.RawURLEncoding.EncodeToString(bytes), nil
+	return bytes, base64.RawURLEncoding.EncodeToString(bytes), nil
 }
 
 // NewSecretKey generate a new value intended to be used by SECRET_KEY.

diff --git a/modules/setting/lfs.go b/modules/setting/lfs.go
@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"code.gitea.io/gitea/modules/generate"
+	"code.gitea.io/gitea/modules/util"
 )
 
 // LFS represents the configuration for Git LFS
@@ -56,17 +57,14 @@ func loadLFSFrom(rootCfg ConfigProvider) error {
 
 	LFS.HTTPAuthExpiry = sec.Key("LFS_HTTP_AUTH_EXPIRY").MustDuration(24 * time.Hour)
 
-	if !LFS.StartServer {
+	if !LFS.StartServer || !InstallLock {
 		return nil
 	}
 
 	LFS.JWTSecretBase64 = loadSecret(rootCfg.Section("server"), "LFS_JWT_SECRET_URI", "LFS_JWT_SECRET")
-
-	LFS.JWTSecretBytes = make([]byte, 32)
-	n, err := base64.RawURLEncoding.Decode(LFS.JWTSecretBytes, []byte(LFS.JWTSecretBase64))
-
-	if (err != nil || n != 32) && InstallLock {
-		LFS.JWTSecretBase64, err = generate.NewJwtSecretBase64()
+	LFS.JWTSecretBytes, err = util.Base64FixedDecode(base64.RawURLEncoding, []byte(LFS.JWTSecretBase64), 32)
+	if err != nil {
+		LFS.JWTSecretBytes, LFS.JWTSecretBase64, err = generate.NewJwtSecretBase64()
 		if err != nil {
 			return fmt.Errorf("error generating JWT Secret for custom config: %v", err)
 		}

diff --git a/modules/setting/oauth2.go b/modules/setting/oauth2.go
@@ -10,6 +10,7 @@ import (
 
 	"code.gitea.io/gitea/modules/generate"
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/util"
 )
 
 // OAuth2UsernameType is enum describing the way gitea 'name' should be generated from oauth2 data
@@ -129,21 +130,19 @@ func loadOAuth2From(rootCfg ConfigProvider) {
 	}
 
 	if InstallLock {
-		key := make([]byte, 32)
-		n, err := base64.RawURLEncoding.Decode(key, []byte(OAuth2.JWTSecretBase64))
-		if err != nil || n != 32 {
-			key, err = generate.NewJwtSecret()
+		if _, err := util.Base64FixedDecode(base64.RawURLEncoding, []byte(OAuth2.JWTSecretBase64), 32); err != nil {
+			key, err := generate.NewJwtSecret()
 			if err != nil {
 				log.Fatal("error generating JWT secret: %v", err)
 			}
 
-			secretBase64 := base64.RawURLEncoding.EncodeToString(key)
+			OAuth2.JWTSecretBase64 = base64.RawURLEncoding.EncodeToString(key)
 			saveCfg, err := rootCfg.PrepareSaving()
 			if err != nil {
 				log.Fatal("save oauth2.JWT_SECRET failed: %v", err)
 			}
-			rootCfg.Section("oauth2").Key("JWT_SECRET").SetValue(secretBase64)
-			saveCfg.Section("oauth2").Key("JWT_SECRET").SetValue(secretBase64)
+			rootCfg.Section("oauth2").Key("JWT_SECRET").SetValue(OAuth2.JWTSecretBase64)
+			saveCfg.Section("oauth2").Key("JWT_SECRET").SetValue(OAuth2.JWTSecretBase64)
 			if err := saveCfg.Save(); err != nil {
 				log.Fatal("save oauth2.JWT_SECRET failed: %v", err)
 			}

diff --git a/modules/util/util.go b/modules/util/util.go
@@ -6,6 +6,7 @@ package util
 import (
 	"bytes"
 	"crypto/rand"
+	"encoding/base64"
 	"fmt"
 	"math/big"
 	"strconv"
@@ -261,3 +262,13 @@ func ToFloat64(number any) (float64, error) {
 func ToPointer[T any](val T) *T {
 	return &val
 }
+
+func Base64FixedDecode(encoding *base64.Encoding, src []byte, length int) ([]byte, error) {
+	decoded := make([]byte, encoding.DecodedLen(len(src))+3)
+	if n, err := encoding.Decode(decoded, src); err != nil {
+		return nil, err
+	} else if n != length {
+		return nil, fmt.Errorf("invalid base64 decoded length: %d, expects: %d", n, length)
+	}
+	return decoded[:length], nil
+}
diff --git a/modules/util/util_test.go b/modules/util/util_test.go
@@ -4,6 +4,7 @@
 package util
 
 import (
+	"encoding/base64"
 	"regexp"
 	"strings"
 	"testing"
@@ -233,3 +234,16 @@ func TestToPointer(t *testing.T) {
 	val123 := 123
 	assert.False(t, &val123 == ToPointer(val123))
 }
+
+func TestBase64FixedDecode(t *testing.T) {
+	_, err := Base64FixedDecode(base64.RawURLEncoding, []byte("abcd"), 32)
+	assert.ErrorContains(t, err, "invalid base64 decoded length")
+	_, err = Base64FixedDecode(base64.RawURLEncoding, []byte(strings.Repeat("a", 64)), 32)
+	assert.ErrorContains(t, err, "invalid base64 decoded length")
+
+	str32 := strings.Repeat("x", 32)
+	encoded32 := base64.RawURLEncoding.EncodeToString([]byte(str32))
+	decoded32, err := Base64FixedDecode(base64.RawURLEncoding, []byte(encoded32), 32)
+	assert.NoError(t, err)
+	assert.Equal(t, str32, string(decoded32))
+}
diff --git a/routers/install/install.go b/routers/install/install.go
@@ -415,7 +415,7 @@ func SubmitInstall(ctx *context.Context) {
 		cfg.Section("server").Key("LFS_START_SERVER").SetValue("true")
 		cfg.Section("lfs").Key("PATH").SetValue(form.LFSRootPath)
 		var lfsJwtSecret string
-		if lfsJwtSecret, err = generate.NewJwtSecretBase64(); err != nil {
+		if _, lfsJwtSecret, err = generate.NewJwtSecretBase64(); err != nil {
 			ctx.RenderWithErr(ctx.Tr("install.lfs_jwt_secret_failed", err), tplInstall, &form)
 			return
 		}

diff --git a/services/auth/source/oauth2/jwtsigningkey.go b/services/auth/source/oauth2/jwtsigningkey.go
@@ -336,16 +336,7 @@ func InitSigningKey() error {
 // loadSymmetricKey checks if the configured secret is valid.
 // If it is not valid, it will return an error.
 func loadSymmetricKey() (any, error) {
-	key := make([]byte, 32)
-	n, err := base64.RawURLEncoding.Decode(key, []byte(setting.OAuth2.JWTSecretBase64))
-	if err != nil {
-		return nil, err
-	}
-	if n != 32 {
-		return nil, fmt.Errorf("JWT secret must be 32 bytes long")
-	}
-
-	return key, nil
+	return util.Base64FixedDecode(base64.RawURLEncoding, []byte(setting.OAuth2.JWTSecretBase64), 32)
 }
 
 // loadOrCreateAsymmetricKey checks if the configured private key exists.