Browse Source

fix possible disclosure

Unknwon 9 years ago
parent
commit
3362b3a44f
7 changed files with 61 additions and 21 deletions
  1. 1 1
      README.md
  2. 7 6
      cmd/serve.go
  3. 1 1
      gogs.go
  4. 37 6
      models/migrations/migrations.go
  5. 2 0
      models/org.go
  6. 12 6
      routers/repo/pull.go
  7. 1 1
      templates/.VERSION

+ 1 - 1
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  |
 |:-------------:|:-------:|:-------:|

+ 7 - 6
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.

+ 1 - 1
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())

+ 37 - 6
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()
+}

+ 2 - 0
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

+ 12 - 6
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)

+ 1 - 1
templates/.VERSION

@@ -1 +1 @@
-0.8.4.1214
+0.8.5.1214