error 500 on login - invalid email address

[fnetX]

Description

A user on Codeberg reported they cannot login any more, because they only see an error 500 screen. We were able to reproduce: Users with an email address that is considered invalid by Gitea after it was registered have trouble signin in. It was _@<hostname> in this case.

2022/06/05 21:16:13 ...ers/web/auth/auth.go:354:handleSignInFull() [E] UpdateUserCols: e-mail invalid [email: _@<hostname>]

It seems to work after reloading the page, but is still worrisome. Please confirm that users who do have valid email addresses that aren't considered valid by Gitea any more are not limited in their actions (e.g. receiving notifications, resetting password, etc).

I can only confirm that the mail address is still rejected by recent Gitea versions.

Further, editing the user lead to unhelpful error messages in the admin panel and lead to the "Create User Account" instead of the "edit" view, but I don't know how to provide more information on that weirdness, but I hope this will be fixed, too.

Gitea Version

1.16.8

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

Codeberg, built from slightly modified fork,.

Database

MySQL

[lunny]

From v1.16.4, the first charactor of email should be [a-zA-Z0-9] for security reason, this should be a break change because users may registered email address before this version.

ref: 1cb6495#diff-a3ba59893d1c58125915ec8374a660356a259362e50c2d5dd3b04801dfc291e6R137-R141

[u0nel]

what are the security reasons exactly?

[lunny]

what are the security reasons exactly?

I can't remember that clearly, some reasons are some invisible characters in the email address could be accepted before.

[HermannDppes]

what are the security reasons exactly?

As outlined in #19102, people were passing the addresses incorrectly to sendmail (it is, after all, far to easy to accidentally misuse weakly typed dynamic languages such as those effectively implemented by any command line interface). So, because these people were not escaping the attacker-controlled inputs properly, it was made harder to supply inputs necessitating an escape.

[u0nel]

the email address didn't start with - or -- and didn't include any invisible characters, so the two previous comments don't apply to this case (localpart being _)

[zeripath]

There are a few issues:

1. Ambiguous Characters, None of the following characters: АВСЕНІЈKКМОРЅТХасеіјорѕухЗ are in the latin script.

if #19990 would be merged we could finally begin extending ambiguous character detection throughout Gitea as this provides the correct detection framework to do it. Then we can finally make it clear when this happening.

2. Sendmail - having an initial - in an email address will cause that email address to be interpreted as an option.

Now most sendmail implementations have an -- option to terminate option interpretation but ... I cannot be certain how many do. Generally it's considered bad form to have - as the initial character for this reason.

3. Initial underscores - I guess we could allow these - @u0nel please send a PR.

[balanceofcowards]

Thanks for the work on this. I ran into this issue as well (also having an e-mail address starting with an underscore), and I'd like to comment a few things. NB: The file in question is models/user/email_address.go.

Ambiguous characters such as mentioned by @zeripath are a non-issue here: they are already correctly filtered by the regex in line 125. Indeed, RFC 5322 is pretty clear about the allowed characters in e-mail local parts (cf. RFC5322, pg 13 & 17/18).

The sendmail issue sounds to me like a typical code injection problem. The canonical solution then would be to escape user-provided strings (e.g., by quoting the e-mail address when used as a shell parameter), no? If the sendmail issue cannot be fixed by quoting, please at least limit the damage. The problem is quite specifically the leading hyphen -- it should suffice to check for this character.

The request / bug report here is to accept RFC-compliant e-mail addresses. If the first character of an e-mail address is required to be alphanumeric, then the comment in line 43 becomes misleading: Many standard compliant addresses will be rejected (as is the case, here).

I do not agree that #19903 fixes this bug -- it only provides a warning to server admins and does not indicate in what way an RFC-compliant address is considered invalid. Plus, I still have this problem on gitea 1.17.0 and I do not get a warning using 'gitea doctor'.

I would ask to reopen this issue. Also, I just sent a related PR: #20991

[delvh]

Quote from @zeripath regarding this issue when I asked him about it some time ago:

the main problem was an initial -.
But then, there is the additional issue of ambiguous characters
But that's really a presentational issue.
If #19990 had been merged and backported, we could have solved that problem.
Or at least the ambiguous character parts.
#19990 would only provide the framework for checking for ambiguous characters but extending that to the display of email addresses would have been easy.

[balanceofcowards]

Hm, yes, but as I just argued: Ambiguous characters are a non-issue here. They are already filtered by the regex, and would not be RFC-compliant, anyway. The issue here is RFC-compliant addresses being rejected.