fix(user): unify email validation for registration and settings (#39304)

Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

models/user/email_address.go

@@ -7,7 +7,6 @@ package user
 import (
 	"context"
 	"fmt"
-	"net/mail"
 	"strings"
 	"time"
 
@@ -22,45 +21,6 @@ import (
 	"xorm.io/builder"
 )
 
-// ErrEmailCharIsNotSupported e-mail address contains unsupported character
-type ErrEmailCharIsNotSupported struct {
-	Email string
-}
-
-// IsErrEmailCharIsNotSupported checks if an error is an ErrEmailCharIsNotSupported
-func IsErrEmailCharIsNotSupported(err error) bool {
-	_, ok := err.(ErrEmailCharIsNotSupported)
-	return ok
-}
-
-func (err ErrEmailCharIsNotSupported) Error() string {
-	return fmt.Sprintf("e-mail address contains unsupported character [email: %s]", err.Email)
-}
-
-func (err ErrEmailCharIsNotSupported) Unwrap() error {
-	return util.ErrInvalidArgument
-}
-
-// ErrEmailInvalid represents an error where the email address does not comply with RFC 5322
-// or has a leading '-' character
-type ErrEmailInvalid struct {
-	Email string
-}
-
-// IsErrEmailInvalid checks if an error is an ErrEmailInvalid
-func IsErrEmailInvalid(err error) bool {
-	_, ok := err.(ErrEmailInvalid)
-	return ok
-}
-
-func (err ErrEmailInvalid) Error() string {
-	return fmt.Sprintf("e-mail invalid [email: %s]", err.Email)
-}
-
-func (err ErrEmailInvalid) Unwrap() error {
-	return util.ErrInvalidArgument
-}
-
 // ErrEmailAlreadyUsed represents a "EmailAlreadyUsed" kind of error.
 type ErrEmailAlreadyUsed struct {
 	Email string
@@ -147,18 +107,34 @@ func InsertEmailAddress(ctx context.Context, email *EmailAddress) (*EmailAddress
 	return email, nil
 }
 
+type ErrEmailInvalid string
+
+func (err ErrEmailInvalid) Error() string {
+	return string(err)
+}
+
+func (err ErrEmailInvalid) Unwrap() error {
+	return util.ErrInvalidArgument
+}
+
 // ValidateEmail check if email is a valid & allowed address
 func ValidateEmail(email string) error {
-	if err := validateEmailBasic(email); err != nil {
-		return err
+	if !validation.IsEmailAddressValid(email) {
+		return ErrEmailInvalid("email address is invalid: " + email)
 	}
-	return validateEmailDomain(email)
+	if !IsEmailDomainAllowed(email) {
+		return ErrEmailInvalid("email domain is not allowed: " + email)
+	}
+	return nil
 }
 
 // ValidateEmailForAdmin check if email is a valid address when admins manually add or edit users
 func ValidateEmailForAdmin(email string) error {
-	return validateEmailBasic(email)
 	// In this case we do not need to check the email domain
+	if !validation.IsEmailAddressValid(email) {
+		return ErrEmailInvalid("email address is invalid: " + email)
+	}
+	return nil
 }
 
 func GetEmailAddressByEmail(ctx context.Context, email string) (*EmailAddress, error) {
@@ -491,37 +467,11 @@ func ActivateUserEmail(ctx context.Context, userID int64, email string, activate
 	})
 }
 
-// validateEmailBasic checks whether the email complies with the rules
-func validateEmailBasic(email string) error {
-	if len(email) == 0 {
-		return ErrEmailInvalid{email}
-	}
-
-	if !globalVars().emailRegexp.MatchString(email) {
-		return ErrEmailCharIsNotSupported{email}
-	}
-
-	if email[0] == '-' {
-		return ErrEmailInvalid{email}
-	}
-
-	if _, err := mail.ParseAddress(email); err != nil {
-		return ErrEmailInvalid{email}
-	}
-
-	return nil
-}
-
-// validateEmailDomain checks whether the email domain is allowed or blocked
-func validateEmailDomain(email string) error {
-	if !IsEmailDomainAllowed(email) {
-		return ErrEmailInvalid{email}
-	}
-
-	return nil
-}
-
 func IsEmailDomainAllowed(email string) bool {
+	localPart, _, _ := strings.CutLast(email, "@")
+	if strings.ContainsAny(localPart, "%!") && (len(setting.Service.EmailDomainAllowList) > 0 || len(setting.Service.EmailDomainBlockList) > 0) {
+		return false // percent-hack and bang-path local parts can route mail to a domain other than the listed one
+	}
 	if len(setting.Service.EmailDomainAllowList) == 0 {
 		return !validation.IsEmailDomainListed(setting.Service.EmailDomainBlockList, email)
 	}

models/user/email_address_test.go

@@ -150,58 +150,22 @@ func TestListEmails(t *testing.T) {
 }
 
 func TestEmailAddressValidate(t *testing.T) {
-	kases := map[string]error{
-		"abc@gmail.com":                  nil,
-		"132@hotmail.com":                nil,
-		"1-3-2@test.org":                 nil,
-		"1.3.2@test.org":                 nil,
-		"a_123@test.org.cn":              nil,
-		`first.last@iana.org`:            nil,
-		`first!last@iana.org`:            nil,
-		`first#last@iana.org`:            nil,
-		`first$last@iana.org`:            nil,
-		`first%last@iana.org`:            nil,
-		`first&last@iana.org`:            nil,
-		`first'last@iana.org`:            nil,
-		`first*last@iana.org`:            nil,
-		`first+last@iana.org`:            nil,
-		`first/last@iana.org`:            nil,
-		`first=last@iana.org`:            nil,
-		`first?last@iana.org`:            nil,
-		`first^last@iana.org`:            nil,
-		"first`last@iana.org":            nil,
-		`first{last@iana.org`:            nil,
-		`first|last@iana.org`:            nil,
-		`first}last@iana.org`:            nil,
-		`first~last@iana.org`:            nil,
-		`first;last@iana.org`:            user_model.ErrEmailCharIsNotSupported{`first;last@iana.org`},
-		".233@qq.com":                    user_model.ErrEmailInvalid{".233@qq.com"},
-		"!233@qq.com":                    nil,
-		"#233@qq.com":                    nil,
-		"$233@qq.com":                    nil,
-		"%233@qq.com":                    nil,
-		"&233@qq.com":                    nil,
-		"'233@qq.com":                    nil,
-		"*233@qq.com":                    nil,
-		"+233@qq.com":                    nil,
-		"-233@qq.com":                    user_model.ErrEmailInvalid{"-233@qq.com"},
-		"/233@qq.com":                    nil,
-		"=233@qq.com":                    nil,
-		"?233@qq.com":                    nil,
-		"^233@qq.com":                    nil,
-		"_233@qq.com":                    nil,
-		"`233@qq.com":                    nil,
-		"{233@qq.com":                    nil,
-		"|233@qq.com":                    nil,
-		"}233@qq.com":                    nil,
-		"~233@qq.com":                    nil,
-		";233@qq.com":                    user_model.ErrEmailCharIsNotSupported{";233@qq.com"},
-		"Foo <foo@bar.com>":              user_model.ErrEmailCharIsNotSupported{"Foo <foo@bar.com>"},
-		string([]byte{0xE2, 0x84, 0xAA}): user_model.ErrEmailCharIsNotSupported{string([]byte{0xE2, 0x84, 0xAA})},
+	cases := map[string]bool{
+		"":                   false,
+		"root@localhost":     true,
+		"user@[192.168.1.2]": true,
+		"@a":                 false,
+		"abc@gmail.com":      true,
+		"abc@gmail.com\n":    false,
+		"Foo <foo@bar.com>":  false,
+		"abc@gmail.com (x)":  false,
+		"jürgen@example.com": false,
+		"a@foo_bar.com":      false,
 	}
-	for kase, err := range kases {
-		t.Run(kase, func(t *testing.T) {
-			assert.Equal(t, err, user_model.ValidateEmail(kase))
+	for tc, isValid := range cases {
+		t.Run(tc, func(t *testing.T) {
+			err := user_model.ValidateEmail(tc)
+			assert.Equal(t, err == nil, isValid)
 		})
 	}
 }

models/user/user.go

@@ -553,7 +553,6 @@ type globalVarsStruct struct {
 	transformDiacritics    transform.Transformer
 	replaceCharsHyphenRE   *regexp.Regexp
 	emailToReplacer        *strings.Replacer
-	emailRegexp            *regexp.Regexp
 	systemUserNewFuncs     map[int64]func() *User
 	systemUserNameIdMap    map[string]int64
 }
@@ -577,7 +576,6 @@ var globalVars = sync.OnceValue(func() *globalVarsStruct {
 			":", "",
 			";", "",
 		),
-		emailRegexp: regexp.MustCompile("^[a-zA-Z0-9.!#$%&'*+-/=?^_`{|}~]*@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$"),
 	}
 
 	userFuncs := []func() *User{NewGhostUser, NewActionsUser, NewDeployKeyUser, NewCliUser, NewAuthSourceUser}

models/user/user_test.go

@@ -297,21 +297,6 @@ func TestDisplayName(t *testing.T) {
 	}
 }
 
-func TestCreateUserInvalidEmail(t *testing.T) {
-	user := &user_model.User{
-		Name:               "GiteaBot",
-		Email:              "GiteaBot@gitea.io\r\n",
-		Passwd:             ";p['////..-++']",
-		IsAdmin:            false,
-		Theme:              setting.UI.DefaultTheme,
-		MustChangePassword: false,
-	}
-
-	err := user_model.CreateUser(t.Context(), user, &user_model.Meta{})
-	assert.Error(t, err)
-	assert.True(t, user_model.IsErrEmailCharIsNotSupported(err))
-}
-
 func TestCreateUserEmailAlreadyUsed(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 

modules/validation/binding.go

@@ -48,8 +48,21 @@ func newFieldError(field reflect.StructField, cls, msg string) *BindingError {
 	return &BindingError{[]string{field.Name}, cls, msg} //nolint:govet // make sure no missing fields
 }
 
+func AddValidationError(errs BindingErrors, fieldName, errorMsg string) BindingErrors {
+	errs.Add([]string{fieldName}, ErrCustomMessage, errorMsg)
+	return errs
+}
+
 // AddBindingRules adds additional binding rules
 func AddBindingRules(b *binding.Binder) {
+	b.ClearRules("Email")
+	b.AddRuleNonZero("Email", func(_ context.Context, f *binding.ValidationField) *binding.Error {
+		if !IsEmailAddressValid(f.ValueMustString()) {
+			return newFieldError(f.StructField, binding.ERR_EMAIL, "invalid email")
+		}
+		return nil
+	})
+
 	b.AddRuleNonZero("GitRefName", func(ctx context.Context, f *binding.ValidationField) *binding.Error {
 		if !git.IsValidRefPattern(f.ValueMustString()) {
 			return newFieldError(f.StructField, ErrGitRefName, "GitRefName")

modules/validation/binding_test.go

@@ -21,9 +21,16 @@ type (
 		URL          string `form:"ValidUrl" binding:"ValidUrl"`
 		GlobPattern  string `form:"GlobPattern" binding:"GlobPattern"`
 		RegexPattern string `form:"RegexPattern" binding:"RegexPattern"`
+		Email        string `form:"Email" binding:"Email"`
 	}
 )
 
 func performValidationTest(t *testing.T, testCase validationTestCase) {
 	assert.Equal(t, testCase.expectedErrors, Binder().Validate(t.Context(), testCase.data))
 }
+
+func TestEmailValidation(t *testing.T) {
+	assert.Nil(t, Binder().Validate(t.Context(), &TestForm{Email: "b@a"}))
+	assert.Equal(t, BindingErrors{{FieldNames: []string{"Email"}, Classification: "EmailError", Message: "invalid email"}},
+		Binder().Validate(t.Context(), &TestForm{Email: "abc"}))
+}

modules/validation/helpers.go

@@ -4,14 +4,18 @@
 package validation
 
 import (
+	"net/mail"
 	"net/url"
 	"regexp"
 	"slices"
 	"strings"
 	"sync"
+	"unicode/utf8"
 
 	"gitea.dev/modules/glob"
 	"gitea.dev/modules/setting"
+
+	"golang.org/x/net/idna"
 )
 
 type globalVarsStruct struct {
@@ -108,3 +112,22 @@ func IsValidBadgeSlug(slug string) bool {
 	vars := globalVars()
 	return vars.validBadgeSlugPattern.MatchString(slug) && !vars.invalidBadgeSlugPattern.MatchString(slug)
 }
+
+func IsEmailAddressValid(email string) bool {
+	if strings.ContainsFunc(email, func(r rune) bool { return r >= utf8.RuneSelf }) {
+		// At the moment, we don't support UTF8 email address. To support it, need to correctly handle IDN/punycode
+		return false
+	}
+	addr, err := mail.ParseAddress(email)
+	if err != nil || addr.Address != email {
+		// email must be parseable, and the "email" string must be the address, no other parts
+		return false
+	}
+	_, domain, _ := strings.Cut(email, "@")
+	if strings.HasPrefix(domain, "[") {
+		// address like "foo@[192.168.1.2]"
+		return true
+	}
+	_, err = idna.Registration.ToASCII(domain)
+	return err == nil
+}

modules/web/middleware/binding.go

@@ -88,11 +88,6 @@ func getRuleBody(field reflect.StructField, ruleName string) string {
 	return ""
 }
 
-func AddValidationError(errs validation.BindingErrors, fieldName, errorMsg string) validation.BindingErrors {
-	errs.Add([]string{fieldName}, validation.ErrCustomMessage, errorMsg)
-	return errs
-}
-
 func getFieldDisplayNameForMessage(f any, l translation.Locale, fieldNames []string) (field reflect.StructField, ok bool, displayName string) {
 	if len(fieldNames) == 0 {
 		return field, false, ""

routers/api/v1/admin/user.go

@@ -74,6 +74,8 @@ func CreateUser(ctx *context.APIContext) {
 	//     "$ref": "#/responses/error"
 	//   "403":
 	//     "$ref": "#/responses/forbidden"
+	//   "409":
+	//     "$ref": "#/responses/error"
 	//   "422":
 	//     "$ref": "#/responses/validationError"
 
@@ -136,17 +138,7 @@ func CreateUser(ctx *context.APIContext) {
 	}
 
 	if err := user_model.AdminCreateUser(ctx, u, &user_model.Meta{}, overwriteDefault); err != nil {
-		if user_model.IsErrUserAlreadyExist(err) ||
-			user_model.IsErrEmailAlreadyUsed(err) ||
-			db.IsErrNameReserved(err) ||
-			db.IsErrNameCharsNotAllowed(err) ||
-			user_model.IsErrEmailCharIsNotSupported(err) ||
-			user_model.IsErrEmailInvalid(err) ||
-			db.IsErrNamePatternNotAllowed(err) {
-			ctx.APIError(http.StatusUnprocessableEntity, err.Error())
-		} else {
-			ctx.APIErrorInternal(err)
-		}
+		ctx.APIErrorAuto(err)
 		return
 	}
 
@@ -191,6 +183,8 @@ func EditUser(ctx *context.APIContext) {
 	//     "$ref": "#/responses/error"
 	//   "403":
 	//     "$ref": "#/responses/forbidden"
+	//   "409":
+	//     "$ref": "#/responses/error"
 	//   "422":
 	//     "$ref": "#/responses/validationError"
 
@@ -219,17 +213,7 @@ func EditUser(ctx *context.APIContext) {
 
 	if form.Email != nil {
 		if err := user_service.ReplacePrimaryEmailAddress(ctx, ctx.ContextUser, *form.Email); err != nil {
-			switch {
-			case user_model.IsErrEmailCharIsNotSupported(err), user_model.IsErrEmailInvalid(err):
-				if !user_model.IsEmailDomainAllowed(*form.Email) {
-					err = fmt.Errorf("the domain of user email %s conflicts with EMAIL_DOMAIN_ALLOWLIST or EMAIL_DOMAIN_BLOCKLIST", *form.Email)
-				}
-				ctx.APIError(http.StatusBadRequest, err.Error())
-			case user_model.IsErrEmailAlreadyUsed(err):
-				ctx.APIError(http.StatusBadRequest, err.Error())
-			default:
-				ctx.APIErrorInternal(err)
-			}
+			ctx.APIErrorAuto(err)
 			return
 		}
 	}

routers/api/v1/user/email.go

@@ -4,7 +4,6 @@
 package user
 
 import (
-	"fmt"
 	"net/http"
 
 	user_model "gitea.dev/models/user"
@@ -55,6 +54,10 @@ func AddEmail(ctx *context.APIContext) {
 	// responses:
 	//   '201':
 	//     "$ref": "#/responses/EmailList"
+	//   "400":
+	//     "$ref": "#/responses/error"
+	//   "409":
+	//     "$ref": "#/responses/error"
 	//   "422":
 	//     "$ref": "#/responses/validationError"
 
@@ -70,22 +73,7 @@ func AddEmail(ctx *context.APIContext) {
 	}
 
 	if err := user_service.AddEmailAddresses(ctx, ctx.Doer, form.Emails); err != nil {
-		if errEmailAlreadyUsed, ok := err.(user_model.ErrEmailAlreadyUsed); ok {
-			ctx.APIError(http.StatusUnprocessableEntity, "Email address has been used: "+errEmailAlreadyUsed.Email)
-		} else if user_model.IsErrEmailCharIsNotSupported(err) || user_model.IsErrEmailInvalid(err) {
-			email := ""
-			if typedError, ok := err.(user_model.ErrEmailInvalid); ok {
-				email = typedError.Email
-			}
-			if typedError, ok := err.(user_model.ErrEmailCharIsNotSupported); ok {
-				email = typedError.Email
-			}
-
-			errMsg := fmt.Sprintf("Email address %q invalid", email)
-			ctx.APIError(http.StatusUnprocessableEntity, errMsg)
-		} else {
-			ctx.APIErrorInternal(err)
-		}
+		ctx.APIErrorAuto(err)
 		return
 	}
 

routers/web/admin/users.go

@@ -23,6 +23,7 @@ import (
 	"gitea.dev/modules/optional"
 	"gitea.dev/modules/setting"
 	"gitea.dev/modules/templates"
+	"gitea.dev/modules/util"
 	"gitea.dev/modules/web"
 	"gitea.dev/routers/web/explore"
 	user_setting "gitea.dev/routers/web/user/setting"
@@ -176,6 +177,7 @@ func NewUserPost(ctx *context.Context) {
 		var errNameReserved db.ErrNameReserved
 		var errNamePatternNotAllowed db.ErrNamePatternNotAllowed
 		var errNameCharsNotAllowed db.ErrNameCharsNotAllowed
+		var errEmailInvalid user_model.ErrEmailInvalid
 		switch {
 		case user_model.IsErrUserAlreadyExist(err):
 			ctx.Data["Err_UserName"] = true
@@ -183,7 +185,7 @@ func NewUserPost(ctx *context.Context) {
 		case user_model.IsErrEmailAlreadyUsed(err):
 			ctx.Data["Err_Email"] = true
 			ctx.RenderWithErrDeprecated(ctx.Tr("form.email_been_used"), tplUserNew, &form)
-		case user_model.IsErrEmailInvalid(err), user_model.IsErrEmailCharIsNotSupported(err):
+		case errors.As(err, &errEmailInvalid):
 			ctx.Data["Err_Email"] = true
 			ctx.RenderWithErrDeprecated(ctx.Tr("form.email_invalid"), tplUserNew, &form)
 		case errors.As(err, &errNameReserved):
@@ -409,12 +411,12 @@ func EditUserPost(ctx *context.Context) {
 	if form.Email != "" {
 		if err := user_service.ReplacePrimaryEmailAddress(ctx, u, form.Email); err != nil {
 			switch {
-			case user_model.IsErrEmailCharIsNotSupported(err), user_model.IsErrEmailInvalid(err):
-				ctx.Data["Err_Email"] = true
-				ctx.RenderWithErrDeprecated(ctx.Tr("form.email_invalid"), tplUserEdit, &form)
 			case user_model.IsErrEmailAlreadyUsed(err):
 				ctx.Data["Err_Email"] = true
 				ctx.RenderWithErrDeprecated(ctx.Tr("form.email_been_used"), tplUserEdit, &form)
+			case errors.Is(err, util.ErrInvalidArgument):
+				ctx.Data["Err_Email"] = true
+				ctx.RenderWithErrDeprecated(ctx.Tr("form.email_invalid"), tplUserEdit, &form)
 			default:
 				ctx.ServerError("AddOrSetPrimaryEmailAddress", err)
 			}

routers/web/auth/auth.go

@@ -659,6 +659,7 @@ func createUserInContext(ctx *context.Context, tpl templates.TplName, form any,
 		var errNameReserved db.ErrNameReserved
 		var errNamePatternNotAllowed db.ErrNamePatternNotAllowed
 		var errNameCharsNotAllowed db.ErrNameCharsNotAllowed
+		var errEmailInvalid user_model.ErrEmailInvalid
 		switch {
 		case user_model.IsErrUserAlreadyExist(err):
 			ctx.Data["Err_UserName"] = true
@@ -666,10 +667,7 @@ func createUserInContext(ctx *context.Context, tpl templates.TplName, form any,
 		case user_model.IsErrEmailAlreadyUsed(err):
 			ctx.Data["Err_Email"] = true
 			ctx.RenderWithErrDeprecated(ctx.Tr("form.email_been_used"), tpl, form)
-		case user_model.IsErrEmailCharIsNotSupported(err):
-			ctx.Data["Err_Email"] = true
-			ctx.RenderWithErrDeprecated(ctx.Tr("form.email_invalid"), tpl, form)
-		case user_model.IsErrEmailInvalid(err):
+		case errors.As(err, &errEmailInvalid):
 			ctx.Data["Err_Email"] = true
 			ctx.RenderWithErrDeprecated(ctx.Tr("form.email_invalid"), tpl, form)
 		case errors.As(err, &errNameReserved):

routers/web/user/setting/account.go

@@ -19,6 +19,7 @@ import (
 	"gitea.dev/modules/setting"
 	"gitea.dev/modules/templates"
 	"gitea.dev/modules/timeutil"
+	"gitea.dev/modules/util"
 	"gitea.dev/modules/web"
 	"gitea.dev/services/auth"
 	"gitea.dev/services/auth/source/db"
@@ -187,7 +188,7 @@ func EmailPost(ctx *context.Context) {
 			loadAccountData(ctx)
 
 			ctx.RenderWithErrDeprecated(ctx.Tr("form.email_been_used"), tplSettingsAccount, &form)
-		} else if user_model.IsErrEmailCharIsNotSupported(err) || user_model.IsErrEmailInvalid(err) {
+		} else if errors.Is(err, util.ErrInvalidArgument) {
 			loadAccountData(ctx)
 
 			ctx.RenderWithErrDeprecated(ctx.Tr("form.email_invalid"), tplSettingsAccount, &form)

services/forms/repo_form.go

@@ -264,7 +264,7 @@ type NewSlackHookForm struct {
 
 func (f *NewSlackHookForm) Validate(ctx *middleware.ValidateContext, errs validation.BindingErrors) validation.BindingErrors {
 	if !webhook.IsValidSlackChannel(strings.TrimSpace(f.Channel)) {
-		errs = middleware.AddValidationError(errs, "Channel", ctx.Locale.TrString("repo.settings.add_webhook.invalid_channel_name"))
+		errs = validation.AddValidationError(errs, "Channel", ctx.Locale.TrString("repo.settings.add_webhook.invalid_channel_name"))
 	}
 	return errs
 }

services/forms/user_form.go

@@ -276,7 +276,7 @@ func DetectInvalidOAuth2ApplicationRedirectURI(uris []string) (invalidURL string
 func (f *EditOAuth2ApplicationForm) Validate(ctx *middleware.ValidateContext, errs validation.BindingErrors) validation.BindingErrors {
 	invalidURI := DetectInvalidOAuth2ApplicationRedirectURI(util.SplitTrimSpace(f.RedirectURIs, "\n"))
 	if invalidURI != "" {
-		errs = middleware.AddValidationError(errs, "RedirectURIs", "RedirectURIs: "+ctx.Locale.TrString("form.url_error", `"`+invalidURI+`"`))
+		errs = validation.AddValidationError(errs, "RedirectURIs", "RedirectURIs: "+ctx.Locale.TrString("form.url_error", `"`+invalidURI+`"`))
 	}
 	return errs
 }

services/forms/user_form_test.go

@@ -46,6 +46,7 @@ func TestRegisterForm_IsDomainAllowed_AllowedEmail(t *testing.T) {
 	}{
 		{"security@gitea.io", true},
 		{"security@gITea.io", true},
+		{"hack%evil.example@gitea.io", false},
 		{"invalid", false},
 		{"seee@example.com", false},
 
@@ -69,6 +70,7 @@ func TestRegisterForm_IsDomainAllowed_BlockedEmail(t *testing.T) {
 	}{
 		{"security@gitea.io", false},
 		{"security@gitea.example", true},
+		{"gitea.io!hack@gitea.example", false},
 		{"invalid", true},
 
 		{"user@my.block", false},

services/mailer/sender/sendmail.go

@@ -30,15 +30,10 @@ func (s *SendmailSender) Send(from string, to []string, msg io.WriterTo) error {
 		envelopeFrom = setting.MailService.EnvelopeFrom
 	}
 
-	args := []string{"-f", envelopeFrom, "-i"}
+	// Use "-t" to extract recipients from message headers, don't add email addresses to the command line.
+	// Because email address can start with "-" which can lead to injected command line argument (RCE)
+	args := []string{"-f", envelopeFrom, "-i", "-t"}
 	args = append(args, setting.MailService.SendmailArgs...)
-	for _, recipient := range to {
-		smtpTo, err := sanitizeEmailAddress(recipient)
-		if err != nil {
-			return fmt.Errorf("invalid recipient address %q: %w", recipient, err)
-		}
-		args = append(args, smtpTo)
-	}
 	log.Trace("Sending with: %s %v", setting.MailService.SendmailPath, args)
 
 	desc := fmt.Sprintf("SendMail: %s %v", setting.MailService.SendmailPath, args)

templates/swagger/v1-openapi3.generated.json

@@ -12058,6 +12058,9 @@
           "403": {
             "$ref": "#/components/responses/forbidden"
           },
+          "409": {
+            "$ref": "#/components/responses/error"
+          },
           "422": {
             "$ref": "#/components/responses/validationError"
           }
@@ -12142,6 +12145,9 @@
           "403": {
             "$ref": "#/components/responses/forbidden"
           },
+          "409": {
+            "$ref": "#/components/responses/error"
+          },
           "422": {
             "$ref": "#/components/responses/validationError"
           }
@@ -34999,6 +35005,12 @@
           "201": {
             "$ref": "#/components/responses/EmailList"
           },
+          "400": {
+            "$ref": "#/components/responses/error"
+          },
+          "409": {
+            "$ref": "#/components/responses/error"
+          },
           "422": {
             "$ref": "#/components/responses/validationError"
           }

templates/swagger/v1-swagger.generated.json

@@ -945,6 +945,9 @@
           "403": {
             "$ref": "#/responses/forbidden"
           },
+          "409": {
+            "$ref": "#/responses/error"
+          },
           "422": {
             "$ref": "#/responses/validationError"
           }
@@ -1029,6 +1032,9 @@
           "403": {
             "$ref": "#/responses/forbidden"
           },
+          "409": {
+            "$ref": "#/responses/error"
+          },
           "422": {
             "$ref": "#/responses/validationError"
           }
@@ -22273,6 +22279,12 @@
           "201": {
             "$ref": "#/responses/EmailList"
           },
+          "400": {
+            "$ref": "#/responses/error"
+          },
+          "409": {
+            "$ref": "#/responses/error"
+          },
           "422": {
             "$ref": "#/responses/validationError"
           }

tests/integration/api_admin_test.go

@@ -209,7 +209,7 @@ func TestAPIEditUser(t *testing.T) {
 
 	errMap := make(map[string]any)
 	json.Unmarshal(resp.Body.Bytes(), &errMap)
-	assert.Equal(t, "e-mail invalid [email: ]", errMap["message"])
+	assert.Equal(t, "email address is invalid: ", errMap["message"])
 
 	user2 = unittest.AssertExistsAndLoadBean(t, &user_model.User{LoginName: "user2"})
 	assert.False(t, user2.IsRestricted)
@@ -355,7 +355,7 @@ func TestAPIEditUser_NotAllowedEmailDomain(t *testing.T) {
 	resp := MakeRequest(t, req, http.StatusBadRequest)
 	errMap := make(map[string]string)
 	assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), &errMap))
-	assert.Equal(t, "the domain of user email user2@example1.com conflicts with EMAIL_DOMAIN_ALLOWLIST or EMAIL_DOMAIN_BLOCKLIST", errMap["message"])
+	assert.Equal(t, "email domain is not allowed: user2@example1.com", errMap["message"])
 
 	req = NewRequestWithJSON(t, "PATCH", urlStr, api.EditUserOption{Email: new("user2@example.org")}).AddTokenAuth(token)
 	MakeRequest(t, req, http.StatusOK)

tests/integration/api_user_email_test.go

@@ -76,7 +76,7 @@ func TestAPIAddEmail(t *testing.T) {
 
 	req := NewRequestWithJSON(t, "POST", "/api/v1/user/emails", &opts).
 		AddTokenAuth(token)
-	MakeRequest(t, req, http.StatusUnprocessableEntity)
+	MakeRequest(t, req, http.StatusConflict)
 
 	opts = api.CreateEmailOption{
 		Emails: []string{"user2-3@example.com"},
@@ -109,7 +109,7 @@ func TestAPIAddEmail(t *testing.T) {
 	}
 	req = NewRequestWithJSON(t, "POST", "/api/v1/user/emails", &opts).
 		AddTokenAuth(token)
-	MakeRequest(t, req, http.StatusUnprocessableEntity)
+	MakeRequest(t, req, http.StatusBadRequest)
 }
 
 func TestAPIDeleteEmail(t *testing.T) {