Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/serv panics on git ssh command to non-existent/unauthorized repo #7457

Closed
1 of 3 tasks
aswild opened this issue Jul 13, 2019 · 0 comments · Fixed by #7458
Closed
1 of 3 tasks

cmd/serv panics on git ssh command to non-existent/unauthorized repo #7457

aswild opened this issue Jul 13, 2019 · 0 comments · Fixed by #7458

Comments

@aswild
Copy link
Contributor

aswild commented Jul 13, 2019

  • Gitea version (or commit ref): master (50d8d17)
  • Git version: n/a
  • Operating system: Linux
  • Database: n/a
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL) See description
    • No
    • Not relevant
  • Log gist:

Description

The serv command panics when trying to run a git SSH command on a non-existent or unauthorized repo. A go panic and stack trace is returned to the client, leaking server and build system information.

% git ls-remote ssh://git@try.gitea.io/aswild/test1 
31430109d60ef9dfe85d38b278c997d5db4285d2        HEAD
31430109d60ef9dfe85d38b278c997d5db4285d2        refs/heads/master

% git ls-remote ssh://git@try.gitea.io/aswild/invalid
Gitea: Unauthorized
Cannot find repository aswild/invalid
Gitea: Internal Server Error
Cannot find repository aswild/invalid
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x16531b9]

goroutine 1 [running]:
code.gitea.io/gitea/cmd.runServ(0xc0000cadc0, 0x0, 0x0)
        /go/src/code.gitea.io/gitea/cmd/serv.go:188 +0xa69
github.com/urfave/cli.HandleAction(0x18add20, 0x291ed90, 0xc0000cadc0, 0xc0007f0600, 0x0)
        /go/src/code.gitea.io/gitea/vendor/github.com/urfave/cli/app.go:490 +0xc8
github.com/urfave/cli.Command.Run(0x1b22ea2, 0x4, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1b8bc2b, 0x2f, 0x0, ...)
        /go/src/code.gitea.io/gitea/vendor/github.com/urfave/cli/command.go:210 +0x996
github.com/urfave/cli.(*App).Run(0xc0007fe1a0, 0xc00003c0c0, 0x4, 0x4, 0x0, 0x0)
        /go/src/code.gitea.io/gitea/vendor/github.com/urfave/cli/app.go:255 +0x6af
main.main()
        /go/src/code.gitea.io/gitea/main.go:109 +0x811
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

This looks like a regression in 356854fc5. The log.GitLogger.Fatal call used run os.Exit(1) to exit the serv command, but when that was removed, the function continues normally after the fatal error, causing a panic.
@aswild aswild mentioned this issue Jul 13, 2019
Merged
aswild added a commit to aswild/gitea that referenced this issue Jul 13, 2019
@aswild
cmd/serv: actually exit after fatal errors
41563cf 
Regression in 356854f, where
a log.Fatal call was removed. log.Fatal calls os.Exit(1) as intended,
but without it the fail() function returns normally and execution
continues past the fatal error, causing a panic.

This is visible as a go panic log and stack trace returned to the SSH
client, which is not only ugly, it leaks server and build system
information.

Fix by removing the stray return statement so that the fail() function
always calls os.Exit(1).

Fixes: go-gitea#7457
Signed-off-by: Allen Wild <allenwild93@gmail.com>
aswild added a commit to aswild/gitea that referenced this issue Jul 13, 2019
@aswild
cmd/serv: actuall exit after fatal errors
56ad783 
Regression in 356854f, where
a log.Fatal call was removed. log.Fatal calls os.Exit(1) as intended,
but without it the fail() function returns normally and execution
continues past the fatal error, causing a panic.

This is visible as a go panic log and stack trace returned to the SSH
client, which is not only ugly, it leaks server and build system
information.

Fix by removing the stray return statement so that the fail() function
always calls os.Exit(1).

Fixes: go-gitea#7457
Signed-off-by: Allen Wild <allenwild93@gmail.com>
zeripath pushed a commit that referenced this issue Jul 13, 2019
@aswild @zeripath
cmd/serv: actually exit after fatal errors (#7458)
2186a99 
Regression in 356854f, where
a log.Fatal call was removed. log.Fatal calls os.Exit(1) as intended,
but without it the fail() function returns normally and execution
continues past the fatal error, causing a panic.

This is visible as a go panic log and stack trace returned to the SSH
client, which is not only ugly, it leaks server and build system
information.

Fix by removing the stray return statement so that the fail() function
always calls os.Exit(1).

Fixes: #7457
Signed-off-by: Allen Wild <allenwild93@gmail.com>
aswild added a commit to aswild/gitea that referenced this issue Jul 14, 2019
@aswild
cmd/serv: actually exit after fatal errors
dd9a8cf 
Regression in 356854f, where
a log.Fatal call was removed. log.Fatal calls os.Exit(1) as intended,
but without it the fail() function returns normally and execution
continues past the fatal error, causing a panic.

This is visible as a go panic log and stack trace returned to the SSH
client, which is not only ugly, it leaks server and build system
information.

Fix by removing the stray return statement so that the fail() function
always calls os.Exit(1).

Backport: 2186a99
Fixes: go-gitea#7457
Signed-off-by: Allen Wild <allenwild93@gmail.com>
@aswild aswild mentioned this issue Jul 14, 2019
Merged
lunny pushed a commit that referenced this issue Jul 14, 2019
@aswild @lunny
cmd/serv: actually exit after fatal errors (#7460)
ee1a8d7 
Regression in 356854f, where
a log.Fatal call was removed. log.Fatal calls os.Exit(1) as intended,
but without it the fail() function returns normally and execution
continues past the fatal error, causing a panic.

This is visible as a go panic log and stack trace returned to the SSH
client, which is not only ugly, it leaks server and build system
information.

Fix by removing the stray return statement so that the fail() function
always calls os.Exit(1).

Backport: 2186a99
Fixes: #7457
Signed-off-by: Allen Wild <allenwild93@gmail.com>
jeffliu27 pushed a commit to jeffliu27/gitea that referenced this issue Jul 18, 2019
@aswild @jeffliu27
cmd/serv: actually exit after fatal errors (go-gitea#7458)
5bd2e10 
Regression in 356854f, where
a log.Fatal call was removed. log.Fatal calls os.Exit(1) as intended,
but without it the fail() function returns normally and execution
continues past the fatal error, causing a panic.

This is visible as a go panic log and stack trace returned to the SSH
client, which is not only ugly, it leaks server and build system
information.

Fix by removing the stray return statement so that the fail() function
always calls os.Exit(1).

Fixes: go-gitea#7457
Signed-off-by: Allen Wild <allenwild93@gmail.com>
aswild added a commit to aswild/gitea that referenced this issue Jul 20, 2019
@aswild
cmd/serv: actuall exit after fatal errors
6b557e9 
Regression in 356854f, where
a log.Fatal call was removed. log.Fatal calls os.Exit(1) as intended,
but without it the fail() function returns normally and execution
continues past the fatal error, causing a panic.

This is visible as a go panic log and stack trace returned to the SSH
client, which is not only ugly, it leaks server and build system
information.

Fix by removing the stray return statement so that the fail() function
always calls os.Exit(1).

Fixes: go-gitea#7457
Signed-off-by: Allen Wild <allenwild93@gmail.com>
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

cmd/serv: actually exit after fatal errors aswild/gitea
1 participant
@aswild