Restrict email address validation (go-gitea#17688)

This didn't follow the RFC but it's a subset of that. I think we should narrow the allowed chars at first and discuss more possibility in future PRs.

models/user/email_address.go

@@ -10,6 +10,7 @@ import (
  "errors"
  "fmt"
  "net/mail"
+ "regexp"
  "strings"
 
  "code.gitea.io/gitea/models/db"
@@ -22,7 +23,22 @@ import (
 )
 
 // ErrEmailNotActivated e-mail address has not been activated error
-var ErrEmailNotActivated = errors.New("E-mail address has not been activated")
+var ErrEmailNotActivated = errors.New("e-mail address has not been activated")
+
+// 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)
+}
 
 // ErrEmailInvalid represents an error where the email address does not comply with RFC 5322
 type ErrEmailInvalid struct {
@@ -106,12 +122,24 @@ func (email *EmailAddress) BeforeInsert() {
  }
 }
 
+var 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])?)*$")
+
 // ValidateEmail check if email is a allowed address
 func ValidateEmail(email string) error {
  if len(email) == 0 {
  return nil
  }
 
+ if !emailRegexp.MatchString(email) {
+ return ErrEmailCharIsNotSupported{email}
+ }
+
+ if !(email[0] >= 'a' && email[0] <= 'z') &&
+ !(email[0] >= 'A' && email[0] <= 'Z') &&
+ !(email[0] >= '0' && email[0] <= '9') {
+ return ErrEmailInvalid{email}
+ }
+
  if _, err := mail.ParseAddress(email); err != nil {
  return ErrEmailInvalid{email}
  }

models/user/email_address_test.go

@@ -252,3 +252,58 @@ func TestListEmails(t *testing.T) {
  assert.Len(t, emails, 5)
  assert.Greater(t, count, int64(len(emails)))
 }
+
+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`: ErrEmailCharIsNotSupported{`first;last@iana.org`},
+ ".233@qq.com": ErrEmailInvalid{".233@qq.com"},
+ "!233@qq.com": ErrEmailInvalid{"!233@qq.com"},
+ "#233@qq.com": ErrEmailInvalid{"#233@qq.com"},
+ "$233@qq.com": ErrEmailInvalid{"$233@qq.com"},
+ "%233@qq.com": ErrEmailInvalid{"%233@qq.com"},
+ "&233@qq.com": ErrEmailInvalid{"&233@qq.com"},
+ "'233@qq.com": ErrEmailInvalid{"'233@qq.com"},
+ "*233@qq.com": ErrEmailInvalid{"*233@qq.com"},
+ "+233@qq.com": ErrEmailInvalid{"+233@qq.com"},
+ "/233@qq.com": ErrEmailInvalid{"/233@qq.com"},
+ "=233@qq.com": ErrEmailInvalid{"=233@qq.com"},
+ "?233@qq.com": ErrEmailInvalid{"?233@qq.com"},
+ "^233@qq.com": ErrEmailInvalid{"^233@qq.com"},
+ "`233@qq.com": ErrEmailInvalid{"`233@qq.com"},
+ "{233@qq.com": ErrEmailInvalid{"{233@qq.com"},
+ "|233@qq.com": ErrEmailInvalid{"|233@qq.com"},
+ "}233@qq.com": ErrEmailInvalid{"}233@qq.com"},
+ "~233@qq.com": ErrEmailInvalid{"~233@qq.com"},
+ ";233@qq.com": ErrEmailCharIsNotSupported{";233@qq.com"},
+ "Foo <foo@bar.com>": ErrEmailCharIsNotSupported{"Foo <foo@bar.com>"},
+ string([]byte{0xE2, 0x84, 0xAA}): ErrEmailCharIsNotSupported{string([]byte{0xE2, 0x84, 0xAA})},
+ }
+ for kase, err := range kases {
+ t.Run(kase, func(t *testing.T) {
+ assert.EqualValues(t, err, ValidateEmail(kase))
+ })
+ }
+}

models/user/user.go

@@ -644,6 +644,15 @@ func CreateUser(u *User, overwriteDefault ...*CreateUserOverwriteOptions) (err e
  u.Visibility = overwriteDefault[0].Visibility
  }
 
+ // validate data
+ if err := validateUser(u); err != nil {
+ return err
+ }
+
+ if err := ValidateEmail(u.Email); err != nil {
+ return err
+ }
+
  ctx, committer, err := db.TxContext()
  if err != nil {
  return err
@@ -652,11 +661,6 @@ func CreateUser(u *User, overwriteDefault ...*CreateUserOverwriteOptions) (err e
 
  sess := db.GetEngine(ctx)
 
- // validate data
- if err := validateUser(u); err != nil {
- return err
- }
-
  isExist, err := isUserExist(sess, 0, u.Name)
  if err != nil {
  return err

models/user/user_test.go

@@ -232,7 +232,7 @@ func TestCreateUserInvalidEmail(t *testing.T) {
 
  err := CreateUser(user)
  assert.Error(t, err)
- assert.True(t, IsErrEmailInvalid(err))
+ assert.True(t, IsErrEmailCharIsNotSupported(err))
 }
 
 func TestCreateUserEmailAlreadyUsed(t *testing.T) {

routers/api/v1/admin/user.go

@@ -119,6 +119,7 @@ func CreateUser(ctx *context.APIContext) {
  user_model.IsErrEmailAlreadyUsed(err) ||
  db.IsErrNameReserved(err) ||
  db.IsErrNameCharsNotAllowed(err) ||
+ user_model.IsErrEmailCharIsNotSupported(err) ||
  user_model.IsErrEmailInvalid(err) ||
  db.IsErrNamePatternNotAllowed(err) {
  ctx.Error(http.StatusUnprocessableEntity, "", err)
@@ -265,7 +266,9 @@ func EditUser(ctx *context.APIContext) {
  }
 
  if err := user_model.UpdateUser(u, emailChanged); err != nil {
- if user_model.IsErrEmailAlreadyUsed(err) || user_model.IsErrEmailInvalid(err) {
+ if user_model.IsErrEmailAlreadyUsed(err) ||
+ user_model.IsErrEmailCharIsNotSupported(err) ||
+ user_model.IsErrEmailInvalid(err) {
  ctx.Error(http.StatusUnprocessableEntity, "", err)
  } else {
  ctx.Error(http.StatusInternalServerError, "UpdateUser", err)

routers/api/v1/user/email.go

@@ -80,7 +80,8 @@ func AddEmail(ctx *context.APIContext) {
  if err := user_model.AddEmailAddresses(emails); err != nil {
  if user_model.IsErrEmailAlreadyUsed(err) {
  ctx.Error(http.StatusUnprocessableEntity, "", "Email address has been used: "+err.(user_model.ErrEmailAlreadyUsed).Email)
- } else if user_model.IsErrEmailInvalid(err) {
+ } else if user_model.IsErrEmailCharIsNotSupported(err) ||
+ user_model.IsErrEmailInvalid(err) {
  errMsg := fmt.Sprintf("Email address %s invalid", err.(user_model.ErrEmailInvalid).Email)
  ctx.Error(http.StatusUnprocessableEntity, "", errMsg)
  } else {

routers/web/admin/users.go

@@ -171,6 +171,9 @@ func NewUserPost(ctx *context.Context) {
  case user_model.IsErrEmailAlreadyUsed(err):
  ctx.Data["Err_Email"] = true
  ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplUserNew, &form)
+ case user_model.IsErrEmailCharIsNotSupported(err):
+ ctx.Data["Err_Email"] = true
+ ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplUserNew, &form)
  case user_model.IsErrEmailInvalid(err):
  ctx.Data["Err_Email"] = true
  ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplUserNew, &form)
@@ -386,7 +389,8 @@ func EditUserPost(ctx *context.Context) {
  if user_model.IsErrEmailAlreadyUsed(err) {
  ctx.Data["Err_Email"] = true
  ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplUserEdit, &form)
- } else if user_model.IsErrEmailInvalid(err) {
+ } else if user_model.IsErrEmailCharIsNotSupported(err) ||
+ user_model.IsErrEmailInvalid(err) {
  ctx.Data["Err_Email"] = true
  ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplUserEdit, &form)
  } else {

routers/web/auth/auth.go

@@ -573,6 +573,9 @@ func createUserInContext(ctx *context.Context, tpl base.TplName, form interface{
  case user_model.IsErrEmailAlreadyUsed(err):
  ctx.Data["Err_Email"] = true
  ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tpl, form)
+ case user_model.IsErrEmailCharIsNotSupported(err):
+ ctx.Data["Err_Email"] = true
+ ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tpl, form)
  case user_model.IsErrEmailInvalid(err):
  ctx.Data["Err_Email"] = true
  ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tpl, form)

routers/web/user/setting/account.go

@@ -188,7 +188,8 @@ func EmailPost(ctx *context.Context) {
 
  ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplSettingsAccount, &form)
  return
- } else if user_model.IsErrEmailInvalid(err) {
+ } else if user_model.IsErrEmailCharIsNotSupported(err) ||
+ user_model.IsErrEmailInvalid(err) {
  loadAccountData(ctx)
 
  ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplSettingsAccount, &form)