Skip to content

Commit

Permalink
Restrict email address validation (go-gitea#17688)
Browse files Browse the repository at this point in the history
  • Loading branch information
lunny authored and 6543 committed Mar 14, 2022
1 parent 99861e3 commit 89dad55
Show file tree
Hide file tree
Showing 9 changed files with 111 additions and 14 deletions.
34 changes: 30 additions & 4 deletions models/user/email_address.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"errors"
"fmt"
"net/mail"
"regexp"
"strings"

"code.gitea.io/gitea/models/db"
Expand All @@ -21,10 +22,23 @@ import (
"xorm.io/builder"
)

var (
// ErrEmailNotActivated e-mail address has not been activated error
ErrEmailNotActivated = errors.New("E-mail address has not been activated")
)
// ErrEmailNotActivated e-mail address has not been activated error
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 {
Expand Down Expand Up @@ -108,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}
}
Expand Down
55 changes: 55 additions & 0 deletions models/user/email_address_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
"[email protected]": nil,
"[email protected]": nil,
"[email protected]": nil,
"[email protected]": nil,
"[email protected]": nil,
`[email protected]`: nil,
`[email protected]`: nil,
`first#[email protected]`: nil,
`[email protected]`: nil,
`first%[email protected]`: nil,
`first&[email protected]`: nil,
`first'[email protected]`: nil,
`first*[email protected]`: nil,
`[email protected]`: nil,
`first/[email protected]`: nil,
`[email protected]`: nil,
`[email protected]`: nil,
`first^[email protected]`: nil,
"first`[email protected]": nil,
`first{[email protected]`: nil,
`first|[email protected]`: nil,
`first}[email protected]`: nil,
`[email protected]`: nil,
`first;[email protected]`: ErrEmailCharIsNotSupported{`first;[email protected]`},
"[email protected]": ErrEmailInvalid{"[email protected]"},
"[email protected]": ErrEmailInvalid{"[email protected]"},
"#[email protected]": ErrEmailInvalid{"#[email protected]"},
"[email protected]": ErrEmailInvalid{"[email protected]"},
"%[email protected]": ErrEmailInvalid{"%[email protected]"},
"&[email protected]": ErrEmailInvalid{"&[email protected]"},
"'[email protected]": ErrEmailInvalid{"'[email protected]"},
"*[email protected]": ErrEmailInvalid{"*[email protected]"},
"[email protected]": ErrEmailInvalid{"[email protected]"},
"/[email protected]": ErrEmailInvalid{"/[email protected]"},
"[email protected]": ErrEmailInvalid{"[email protected]"},
"[email protected]": ErrEmailInvalid{"[email protected]"},
"^[email protected]": ErrEmailInvalid{"^[email protected]"},
"`[email protected]": ErrEmailInvalid{"`[email protected]"},
"{[email protected]": ErrEmailInvalid{"{[email protected]"},
"|[email protected]": ErrEmailInvalid{"|[email protected]"},
"}[email protected]": ErrEmailInvalid{"}[email protected]"},
"[email protected]": ErrEmailInvalid{"[email protected]"},
";[email protected]": ErrEmailCharIsNotSupported{";[email protected]"},
"Foo <[email protected]>": ErrEmailCharIsNotSupported{"Foo <[email protected]>"},
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))
})
}
}
14 changes: 9 additions & 5 deletions models/user/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion models/user/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
5 changes: 4 additions & 1 deletion routers/api/v1/admin/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion routers/api/v1/user/email.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 5 additions & 1 deletion routers/web/admin/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions routers/web/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion routers/web/user/setting/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 89dad55

Please sign in to comment.