From ed1be4ca68daa9782ea65135110799a4bf0697f8 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 14 Aug 2023 18:30:16 +0800 Subject: [PATCH] Handle base64 decoding correctly to avoid panic (#26483) Fix the panic if the "base64 secret" is too long. --- cmd/generate.go | 4 ++-- modules/generate/generate.go | 6 +++--- modules/setting/lfs.go | 12 +++++------- modules/setting/oauth2.go | 13 ++++++------- modules/util/util.go | 11 +++++++++++ modules/util/util_test.go | 14 ++++++++++++++ routers/install/install.go | 2 +- services/auth/source/oauth2/jwtsigningkey.go | 11 +---------- 8 files changed, 43 insertions(+), 30 deletions(-) diff --git a/cmd/generate.go b/cmd/generate.go index 2cff24458d..5922617217 100644 --- 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 index 8b82976e85..ee3c76059b 100644 --- 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 index 21a46a8cce..a5ea537cef 100644 --- 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 index b88070681a..ab82393106 100644 --- 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 index 9d5e6c1e89..cc25539967 100644 --- 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 index c5830ce01c..8509d8aced 100644 --- 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 index aa9c0f4986..99ea7b0738 100644 --- 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 index ff0d426e22..eca0b8b7e1 100644 --- 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.