From 2cd2614eaa40bc25880431f6ea56f9c47a938508 Mon Sep 17 00:00:00 2001 From: John Olheiser <42128690+jolheiser@users.noreply.github.com> Date: Wed, 5 Feb 2020 08:50:06 -0600 Subject: [PATCH] Fix push-create SSH bugs (#10145) (#10151) * Attempt to fix push-create SSH bugs Signed-off-by: jolheiser * Fix binding Signed-off-by: jolheiser * Invalid ctx Signed-off-by: jolheiser --- cmd/serv.go | 6 ++++++ integrations/git_test.go | 16 ++++++++++++++++ routers/private/serv.go | 12 +++++++++++- 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/cmd/serv.go b/cmd/serv.go index 8ffe9b3a4f5..fa1e0333917 100644 --- a/cmd/serv.go +++ b/cmd/serv.go @@ -12,6 +12,7 @@ import ( "net/url" "os" "os/exec" + "regexp" "strconv" "strings" "time" @@ -72,6 +73,7 @@ var ( "git-receive-pack": models.AccessModeWrite, lfsAuthenticateVerb: models.AccessModeNone, } + alphaDashDotPattern = regexp.MustCompile(`[^\w-\.]`) ) func fail(userMessage, logMessage string, args ...interface{}) { @@ -147,6 +149,10 @@ func runServ(c *cli.Context) error { username := strings.ToLower(rr[0]) reponame := strings.ToLower(strings.TrimSuffix(rr[1], ".git")) + if alphaDashDotPattern.MatchString(reponame) { + fail("Invalid repo name", "Invalid repo name: %s", reponame) + } + if setting.EnablePprof || c.Bool("enable-pprof") { if err := os.MkdirAll(setting.PprofDataPath, os.ModePerm); err != nil { fail("Error while trying to create PPROF_DATA_PATH", "Error while trying to create PPROF_DATA_PATH: %v", err) diff --git a/integrations/git_test.go b/integrations/git_test.go index 7d37555f061..7753b2cb726 100644 --- a/integrations/git_test.go +++ b/integrations/git_test.go @@ -422,6 +422,9 @@ func doPushCreate(ctx APITestContext, u *url.URL) func(t *testing.T) { tmpDir, err := ioutil.TempDir("", ctx.Reponame) assert.NoError(t, err) + _, err = git.NewCommand("clone", u.String()).RunInDir(tmpDir) + assert.Error(t, err) + err = git.InitRepository(tmpDir, false) assert.NoError(t, err) @@ -449,6 +452,13 @@ func doPushCreate(ctx APITestContext, u *url.URL) func(t *testing.T) { _, err = git.NewCommand("remote", "add", "origin", u.String()).RunInDir(tmpDir) assert.NoError(t, err) + invalidCtx := ctx + invalidCtx.Reponame = fmt.Sprintf("invalid/repo-tmp-push-create-%s", u.Scheme) + u.Path = invalidCtx.GitPath() + + _, err = git.NewCommand("remote", "add", "invalid", u.String()).RunInDir(tmpDir) + assert.NoError(t, err) + // Push to create disabled setting.Repository.EnablePushCreateUser = false _, err = git.NewCommand("push", "origin", "master").RunInDir(tmpDir) @@ -456,6 +466,12 @@ func doPushCreate(ctx APITestContext, u *url.URL) func(t *testing.T) { // Push to create enabled setting.Repository.EnablePushCreateUser = true + + // Invalid repo + _, err = git.NewCommand("push", "invalid", "master").RunInDir(tmpDir) + assert.Error(t, err) + + // Valid repo _, err = git.NewCommand("push", "origin", "master").RunInDir(tmpDir) assert.NoError(t, err) diff --git a/routers/private/serv.go b/routers/private/serv.go index c769f30d07c..91c28143eee 100644 --- a/routers/private/serv.go +++ b/routers/private/serv.go @@ -68,7 +68,6 @@ func ServNoCommand(ctx *macaron.Context) { // ServCommand returns information about the provided keyid func ServCommand(ctx *macaron.Context) { - // Although we provide the verbs we don't need them at present they're just for logging purposes keyID := ctx.ParamsInt64(":keyid") ownerName := ctx.Params(":owner") repoName := ctx.Params(":repo") @@ -105,6 +104,17 @@ func ServCommand(ctx *macaron.Context) { if err != nil { if models.IsErrRepoNotExist(err) { repoExist = false + for _, verb := range ctx.QueryStrings("verb") { + if "git-upload-pack" == verb { + // User is fetching/cloning a non-existent repository + ctx.JSON(http.StatusNotFound, map[string]interface{}{ + "results": results, + "type": "ErrRepoNotExist", + "err": fmt.Sprintf("Cannot find repository: %s/%s", results.OwnerName, results.RepoName), + }) + return + } + } } else { log.Error("Unable to get repository: %s/%s Error: %v", results.OwnerName, results.RepoName, err) ctx.JSON(http.StatusInternalServerError, map[string]interface{}{