From 3362b3a44f9f4e0e482b08151e298f7809eefc59 Mon Sep 17 00:00:00 2001 From: Unknwon Date: Mon, 14 Dec 2015 17:06:54 -0500 Subject: [PATCH] fix possible disclosure --- README.md | 2 +- cmd/serve.go | 13 +++++----- gogs.go | 2 +- models/migrations/migrations.go | 43 ++++++++++++++++++++++++++++----- models/org.go | 2 ++ routers/repo/pull.go | 18 +++++++++----- templates/.VERSION | 2 +- 7 files changed, 61 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 438cba5a..ae279c94 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@ Gogs - Go Git Service [![Build Status](https://travis-ci.org/gogits/gogs.svg?bra ![](https://github.com/gogits/gogs/blob/master/public/img/gogs-large-resize.png?raw=true) -##### Current version: 0.8.4 +##### Current version: 0.8.5 | Web | UI | Preview | |:-------------:|:-------:|:-------:| diff --git a/cmd/serve.go b/cmd/serve.go index 247dbb04..f8a5394f 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -17,6 +17,7 @@ import ( "github.com/codegangsta/cli" "github.com/gogits/gogs/models" + "github.com/gogits/gogs/modules/base" "github.com/gogits/gogs/modules/httplib" "github.com/gogits/gogs/modules/log" "github.com/gogits/gogs/modules/setting" @@ -87,7 +88,7 @@ func fail(userMessage, logMessage string, args ...interface{}) { os.Exit(1) } -func handleUpdateTask(uuid string, user *models.User, username, reponame string, isWiki bool) { +func handleUpdateTask(uuid string, user, repoUser *models.User, reponame string, isWiki bool) { task, err := models.GetUpdateTaskByUUID(uuid) if err != nil { if models.IsErrUpdateTaskNotExist(err) { @@ -104,13 +105,13 @@ func handleUpdateTask(uuid string, user *models.User, username, reponame string, } if err = models.Update(task.RefName, task.OldCommitID, task.NewCommitID, - user.Name, username, reponame, user.Id); err != nil { + user.Name, repoUser.Name, reponame, user.Id); err != nil { log.GitLogger.Error(2, "Update: %v", err) } // Ask for running deliver hook and test pull request tasks. - reqURL := setting.LocalUrl + username + "/" + reponame + "/tasks/trigger?branch=" + - strings.TrimPrefix(task.RefName, "refs/heads/") + reqURL := setting.LocalUrl + repoUser.Name + "/" + reponame + "/tasks/trigger?branch=" + + strings.TrimPrefix(task.RefName, "refs/heads/") + "&secret=" + base.EncodeMD5(repoUser.Salt) log.GitLogger.Trace("Trigger task: %s", reqURL) resp, err := httplib.Head(reqURL).SetTLSClientConfig(&tls.Config{ @@ -163,7 +164,7 @@ func runServ(c *cli.Context) { if models.IsErrUserNotExist(err) { fail("Repository owner does not exist", "Unregistered owner: %s", username) } - fail("Internal error", "Failed to get repository owner(%s): %v", username, err) + fail("Internal error", "Failed to get repository owner (%s): %v", username, err) } repo, err := models.GetRepositoryByName(repoUser.Id, reponame) @@ -266,7 +267,7 @@ func runServ(c *cli.Context) { } if requestedMode == models.ACCESS_MODE_WRITE { - handleUpdateTask(uuid, user, username, reponame, isWiki) + handleUpdateTask(uuid, user, repoUser, reponame, isWiki) } // Update user key activity. diff --git a/gogs.go b/gogs.go index 8b9286f2..85255299 100644 --- a/gogs.go +++ b/gogs.go @@ -18,7 +18,7 @@ import ( "github.com/gogits/gogs/modules/setting" ) -const APP_VER = "0.8.4.1214" +const APP_VER = "0.8.5.1214" func init() { runtime.GOMAXPROCS(runtime.NumCPU()) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index ca215d9f..78729bde 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -18,6 +18,7 @@ import ( "github.com/go-xorm/xorm" "gopkg.in/ini.v1" + "github.com/gogits/gogs/modules/base" "github.com/gogits/gogs/modules/log" "github.com/gogits/gogs/modules/setting" gouuid "github.com/gogits/gogs/modules/uuid" @@ -57,12 +58,13 @@ type Version struct { // If you want to "retire" a migration, remove it from the top of the list and // update _MIN_VER_DB accordingly var migrations = []Migration{ - NewMigration("fix locale file load panic", fixLocaleFileLoadPanic), // V4 -> V5:v0.6.0 - NewMigration("trim action compare URL prefix", trimCommitActionAppUrlPrefix), // V5 -> V6:v0.6.3 - NewMigration("generate issue-label from issue", issueToIssueLabel), // V6 -> V7:v0.6.4 - NewMigration("refactor attachment table", attachmentRefactor), // V7 -> V8:v0.6.4 - NewMigration("rename pull request fields", renamePullRequestFields), // V8 -> V9:v0.6.16 - NewMigration("clean up migrate repo info", cleanUpMigrateRepoInfo), // V9 -> V10:v0.6.20 + NewMigration("fix locale file load panic", fixLocaleFileLoadPanic), // V4 -> V5:v0.6.0 + NewMigration("trim action compare URL prefix", trimCommitActionAppUrlPrefix), // V5 -> V6:v0.6.3 + NewMigration("generate issue-label from issue", issueToIssueLabel), // V6 -> V7:v0.6.4 + NewMigration("refactor attachment table", attachmentRefactor), // V7 -> V8:v0.6.4 + NewMigration("rename pull request fields", renamePullRequestFields), // V8 -> V9:v0.6.16 + NewMigration("clean up migrate repo info", cleanUpMigrateRepoInfo), // V9 -> V10:v0.6.20 + NewMigration("generate rands and salt for organizations", generateOrgRandsAndSalt), // V10 -> V11:v0.8.5 } // Migrate database to current version @@ -422,3 +424,32 @@ func cleanUpMigrateRepoInfo(x *xorm.Engine) (err error) { return nil } + +func generateOrgRandsAndSalt(x *xorm.Engine) (err error) { + type User struct { + ID int64 `xorm:"pk autoincr"` + Rands string `xorm:"VARCHAR(10)"` + Salt string `xorm:"VARCHAR(10)"` + } + + orgs := make([]*User, 0, 10) + if err = x.Where("type=1").And("rands=''").Find(&orgs); err != nil { + return fmt.Errorf("select all organizations: %v", err) + } + + sess := x.NewSession() + defer sessionRelease(sess) + if err = sess.Begin(); err != nil { + return err + } + + for _, org := range orgs { + org.Rands = base.GetRandomString(10) + org.Salt = base.GetRandomString(10) + if _, err = sess.Id(org.ID).Update(org); err != nil { + return err + } + } + + return sess.Commit() +} diff --git a/models/org.go b/models/org.go index 608fd348..6cc951be 100644 --- a/models/org.go +++ b/models/org.go @@ -108,6 +108,8 @@ func CreateOrganization(org, owner *User) (err error) { org.LowerName = strings.ToLower(org.Name) org.FullName = org.Name + org.Rands = GetUserSalt() + org.Salt = GetUserSalt() org.UseCustomAvatar = true org.MaxRepoCreation = -1 org.NumTeams = 1 diff --git a/routers/repo/pull.go b/routers/repo/pull.go index d0c1cb67..38a4c470 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -6,7 +6,6 @@ package repo import ( "container/list" - "errors" "path" "strings" @@ -644,17 +643,24 @@ func CompareAndPullRequestPost(ctx *middleware.Context, form auth.CreateIssueFor } func TriggerTask(ctx *middleware.Context) { - _, repo := parseOwnerAndRepo(ctx) + branch := ctx.Query("branch") + secret := ctx.Query("secret") + if len(branch) == 0 || len(secret) == 0 { + ctx.Error(404) + log.Trace("TriggerTask: branch or secret is empty") + return + } + owner, repo := parseOwnerAndRepo(ctx) if ctx.Written() { return } - branch := ctx.Query("branch") - if len(branch) == 0 { - ctx.Handle(422, "TriggerTask", errors.New("branch is empty")) + if secret != base.EncodeMD5(owner.Salt) { + ctx.Error(404) + log.Trace("TriggerTask [%s/%s]: invalid secret", owner.Name, repo.Name) return } - log.Trace("TriggerTask[%d].(new request): %s", repo.ID, branch) + log.Trace("TriggerTask [%d].(new request): %s", repo.ID, branch) go models.HookQueue.Add(repo.ID) go models.AddTestPullRequestTask(repo.ID, branch) diff --git a/templates/.VERSION b/templates/.VERSION index 6e480531..51625d87 100644 --- a/templates/.VERSION +++ b/templates/.VERSION @@ -1 +1 @@ -0.8.4.1214 \ No newline at end of file +0.8.5.1214 \ No newline at end of file