Internal Server Error when creating a repository in edge case

[Larivact]

I do not have Gitea installed and discovered this very weird bug at https://try.gitea.io, which currently uses af45648.

Description

1. Create an account
2. Go to your account settings and set your Full Name to <Test (it needs to start with a less-than sign)
3. Attempt to create a repository: enter a name and check Initialize Repository (Adds .gitignore, License and README) (this is important it will not fail if you do not check it). Click Create Repository.
4. Internal Server Error 500

[zeripath]

Thanks @Larivact I can confirm this bug.

[zeripath]

The issue is this line:

https://github.com/go-gitea/gitea/blob/master/models/repo.go#L1097

		"git", "commit", fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email),

The author's name isn't being escaped - this appears to be safe despite the potential security problems due to the way the whole thing is passed as a single argument direct to the git binary rather than going through an interpreter - git just then interprets the mess after the = as being the author no matter what is in there. It is quite liberal with what it expects to be a name, the only issues seem to be that there can only be one segment enclosed by <> and that segment had better be an email address.

[zeripath]

There are 4 other places that do the same thing in the code base.

[Larivact]

Thanks for tracking this down. Git does not validate email addresses but expects a name:

$ git commit --author='<a>'
fatal: empty ident name (for <a>) not allowed

So the issue can also be reproduced with a Full Name that's just a space.

[zeripath]

Looking at what git does internally, if you set GIT_AUTHOR_NAME and GIT_AUTHOR_EMAIL it will pass them through this function https://github.com/git/git/blob/master/ident.c#L221 which will remove \n<> and Trim preceding and following whitespace.

[zeripath]

I guess we should do this too. If the name is effectively empty we should use the username.

[zeripath]

I'll have a PR up if this passes the integration tests in a few mins

[Larivact]

Just doing strip and trim would still let <Test slide because of the argument parsing. So we should probably pass name & email via environment variables with ExecDirEnv.

[zeripath]

Take a look at my fix, it's cleverer than that.

[Larivact]

I see. Thanks for the quick and elegant solution.