Internal Server Error when creating a repository in edge case #2777

New Issue

GiteaMirror · 2025-11-02T04:48:08-06:00

GiteaMirror commented

2025-11-02 04:48:08 -06:00

Originally created by @Larivact on GitHub (Jan 19, 2019).

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

Description

Create an account
Go to your account settings and set your Full Name to <Test (it needs to start with a less-than sign)
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.
Internal Server Error 500

Originally created by @Larivact on GitHub (Jan 19, 2019). 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

GiteaMirror added the type/bug label 2025-11-02 04:48:08 -06:00

GiteaMirror closed this issue

2025-11-02 04:48:08 -06:00

GiteaMirror commented

2025-11-02 04:48:09 -06:00

@zeripath commented on GitHub (Jan 19, 2019):

Thanks @Larivact I can confirm this bug.

@zeripath commented on GitHub (Jan 19, 2019): Thanks @Larivact I can confirm this bug.

GiteaMirror commented

2025-11-02 04:48:09 -06:00

@zeripath commented on GitHub (Jan 19, 2019):

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 commented on GitHub (Jan 19, 2019): The issue is this line: https://github.com/go-gitea/gitea/blob/master/models/repo.go#L1097 ```go "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.

GiteaMirror commented

2025-11-02 04:48:09 -06:00

@zeripath commented on GitHub (Jan 19, 2019):

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

@zeripath commented on GitHub (Jan 19, 2019): There are 4 other places that do the same thing in the code base.

GiteaMirror commented

2025-11-02 04:48:09 -06:00

@Larivact commented on GitHub (Jan 19, 2019):

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.

@Larivact commented on GitHub (Jan 19, 2019): 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.

GiteaMirror commented

2025-11-02 04:48:10 -06:00

@zeripath commented on GitHub (Jan 19, 2019):

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 commented on GitHub (Jan 19, 2019): 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.

GiteaMirror commented

2025-11-02 04:48:10 -06:00

@zeripath commented on GitHub (Jan 19, 2019):

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

@zeripath commented on GitHub (Jan 19, 2019): I guess we should do this too. If the name is effectively empty we should use the username.

GiteaMirror commented

2025-11-02 04:48:10 -06:00

@zeripath commented on GitHub (Jan 19, 2019):

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

@zeripath commented on GitHub (Jan 19, 2019): I'll have a PR up if this passes the integration tests in a few mins

GiteaMirror commented

2025-11-02 04:48:10 -06:00

@Larivact commented on GitHub (Jan 19, 2019):

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.

@Larivact commented on GitHub (Jan 19, 2019): 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`.

GiteaMirror commented

2025-11-02 04:48:11 -06:00

@zeripath commented on GitHub (Jan 19, 2019):

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

@zeripath commented on GitHub (Jan 19, 2019): Take a look at my fix, it's cleverer than that.

GiteaMirror commented

2025-11-02 04:48:11 -06:00

@Larivact commented on GitHub (Jan 19, 2019):

I see. Thanks for the quick and elegant solution.

@Larivact commented on GitHub (Jan 19, 2019): I see. Thanks for the quick and elegant solution.

GiteaMirror referenced this issue

2025-11-02 12:14:02 -06:00