Changelog for 1.6.2 (#5567)

There are likely problems remaining with the way that initCommentForm
is creating its elements. I suspect that a malformed avatar url could
be used maliciously.

CHANGELOG.md

@@ -4,6 +4,20 @@ This changelog goes through all the changes that have been made in each release
 without substantial changes to our git log; to see the highlights of what has
 been added to each release, please refer to the [blog](https://blog.gitea.io).
 
+## [1.6.2](https://github.com/go-gitea/gitea/releases/tag/v1.6.2) - 2018-12-21
+* SECURITY
+  * Sanitize uploaded file names (#5571) (#5573)
+  * HTMLEncode user added text (#5570) (#5575)
+* BUGFIXES
+  * Fix indexer reindex bug when gitea restart (#5563) (#5564)
+  * Remove a double slash in the HTTPS redirect with Let's Encrypt (#5537) (#5539)
+  * Fix bug when a read perm user to edit his issue (#5516) (#5534)
+  * Detect force push failure on deletion of protected branches (#5522) (#5531)
+  * Let's Encrypt handler listens on correct port for certificate validation (#5525) (#5527)
+  * Fix forgot deletion of notification when delete repository (#5506) (#5514)
+  * Fix undeleted content when deleting user (#5429) (#5509)
+  * Fix empty wiki (#5504) (#5508)
+
 ## [1.6.1](https://github.com/go-gitea/gitea/releases/tag/v1.6.1) - 2018-12-08
 * BUGFIXES
   * Fix dependent issue searching when gitea is run in subpath (#5392) (#5400)

cmd/hook.go

@@ -112,10 +112,15 @@ func runHookPreReceive(c *cli.Context) error {
 		branchName := strings.TrimPrefix(refFullName, git.BranchPrefix)
 		protectBranch, err := private.GetProtectedBranchBy(repoID, branchName)
 		if err != nil {
-			log.GitLogger.Fatal(2, "retrieve protected branches information failed")
+			fail("Internal error", fmt.Sprintf("retrieve protected branches information failed: %v", err))
 		}
 
 		if protectBranch != nil && protectBranch.IsProtected() {
+			// check and deletion
+			if newCommitID == git.EmptySHA {
+				fail(fmt.Sprintf("branch %s is protected from deletion", branchName), "")
+			}
+
 			// detect force push
 			if git.EmptySHA != oldCommitID {
 				output, err := git.NewCommand("rev-list", "--max-count=1", oldCommitID, "^"+newCommitID).RunInDir(repoPath)
@@ -126,17 +131,12 @@ func runHookPreReceive(c *cli.Context) error {
 				}
 			}
 
-			// check and deletion
-			if newCommitID == git.EmptySHA {
-				fail(fmt.Sprintf("branch %s is protected from deletion", branchName), "")
-			} else {
-				userID, _ := strconv.ParseInt(userIDStr, 10, 64)
-				canPush, err := private.CanUserPush(protectBranch.ID, userID)
-				if err != nil {
-					fail("Internal error", "Fail to detect user can push: %v", err)
-				} else if !canPush {
-					fail(fmt.Sprintf("protected branch %s can not be pushed to", branchName), "")
-				}
+			userID, _ := strconv.ParseInt(userIDStr, 10, 64)
+			canPush, err := private.CanUserPush(protectBranch.ID, userID)
+			if err != nil {
+				fail("Internal error", "Fail to detect user can push: %v", err)
+			} else if !canPush {
+				fail(fmt.Sprintf("protected branch %s can not be pushed to", branchName), "")
 			}
 		}
 	}

cmd/web.go

@@ -80,7 +80,13 @@ func runLetsEncrypt(listenAddr, domain, directory, email string, m http.Handler)
 		Cache:      autocert.DirCache(directory),
 		Email:      email,
 	}
-	go http.ListenAndServe(listenAddr+":"+setting.PortToRedirect, certManager.HTTPHandler(http.HandlerFunc(runLetsEncryptFallbackHandler))) // all traffic coming into HTTP will be redirect to HTTPS automatically (LE HTTP-01 validatio happens here)
+	go func() {
+		log.Info("Running Let's Encrypt handler on %s", setting.HTTPAddr+":"+setting.PortToRedirect)
+		var err = http.ListenAndServe(setting.HTTPAddr+":"+setting.PortToRedirect, certManager.HTTPHandler(http.HandlerFunc(runLetsEncryptFallbackHandler))) // all traffic coming into HTTP will be redirect to HTTPS automatically (LE HTTP-01 validation happens here)
+		if err != nil {
+			log.Fatal(4, "Failed to start the Let's Encrypt handler on port %s: %v", setting.PortToRedirect, err)
+		}
+	}()
 	server := &http.Server{
 		Addr:    listenAddr,
 		Handler: m,
@@ -96,7 +102,10 @@ func runLetsEncryptFallbackHandler(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "Use HTTPS", http.StatusBadRequest)
 		return
 	}
-	target := setting.AppURL + r.URL.RequestURI()
+	// Remove the trailing slash at the end of setting.AppURL, the request
+	// URI always contains a leading slash, which would result in a double
+	// slash
+	target := strings.TrimRight(setting.AppURL, "/") + r.URL.RequestURI()
 	http.Redirect(w, r, target, http.StatusFound)
 }
 

models/repo.go

@@ -1838,6 +1838,7 @@ func DeleteRepository(doer *User, uid, repoID int64) error {
 		&RepoRedirect{RedirectRepoID: repoID},
 		&Webhook{RepoID: repoID},
 		&HookTask{RepoID: repoID},
+		&Notification{RepoID: repoID},
 	); err != nil {
 		return fmt.Errorf("deleteBeans: %v", err)
 	}

models/ssh_key.go

@@ -535,6 +535,7 @@ func DeletePublicKey(doer *User, id int64) (err error) {
 	if err = sess.Commit(); err != nil {
 		return err
 	}
+	sess.Close()
 
 	return RewriteAllPublicKeys()
 }
@@ -543,6 +544,10 @@ func DeletePublicKey(doer *User, id int64) (err error) {
 // Note: x.Iterate does not get latest data after insert/delete, so we have to call this function
 // outside any session scope independently.
 func RewriteAllPublicKeys() error {
+	return rewriteAllPublicKeys(x)
+}
+
+func rewriteAllPublicKeys(e Engine) error {
 	//Don't rewrite key if internal server
 	if setting.SSH.StartBuiltinServer {
 		return nil
@@ -569,7 +574,7 @@ func RewriteAllPublicKeys() error {
 		}
 	}
 
-	err = x.Iterate(new(PublicKey), func(idx int, bean interface{}) (err error) {
+	err = e.Iterate(new(PublicKey), func(idx int, bean interface{}) (err error) {
 		_, err = t.WriteString((bean.(*PublicKey)).AuthorizedString())
 		return err
 	})

models/user.go

@@ -1038,25 +1038,26 @@ func deleteUser(e *xorm.Session, u *User) error {
 		&EmailAddress{UID: u.ID},
 		&UserOpenID{UID: u.ID},
 		&Reaction{UserID: u.ID},
+		&TeamUser{UID: u.ID},
+		&Collaboration{UserID: u.ID},
+		&Stopwatch{UserID: u.ID},
 	); err != nil {
 		return fmt.Errorf("deleteBeans: %v", err)
 	}
 
 	// ***** START: PublicKey *****
-	keys := make([]*PublicKey, 0, 10)
-	if err = e.Find(&keys, &PublicKey{OwnerID: u.ID}); err != nil {
-		return fmt.Errorf("get all public keys: %v", err)
-	}
-
-	keyIDs := make([]int64, len(keys))
-	for i := range keys {
-		keyIDs[i] = keys[i].ID
-	}
-	if err = deletePublicKeys(e, keyIDs...); err != nil {
+	if _, err = e.Delete(&PublicKey{OwnerID: u.ID}); err != nil {
 		return fmt.Errorf("deletePublicKeys: %v", err)
 	}
+	rewriteAllPublicKeys(e)
 	// ***** END: PublicKey *****
 
+	// ***** START: GPGPublicKey *****
+	if _, err = e.Delete(&GPGKey{OwnerID: u.ID}); err != nil {
+		return fmt.Errorf("deleteGPGKeys: %v", err)
+	}
+	// ***** END: GPGPublicKey *****
+
 	// Clear assignee.
 	if err = clearAssigneeByUserID(e, u.ID); err != nil {
 		return fmt.Errorf("clear assignee: %v", err)
@@ -1110,6 +1111,7 @@ func DeleteUser(u *User) (err error) {
 	if err = sess.Commit(); err != nil {
 		return err
 	}
+	sess.Close()
 
 	return RewriteAllPublicKeys()
 }

modules/indexer/issue.go

@@ -60,7 +60,7 @@ func InitIssueIndexer(populateIndexer func() error) {
 		return
 	}
 
-	if err = createIssueIndexer(); err != nil {
+	if err = createIssueIndexer(setting.Indexer.IssuePath, issueIndexerLatestVersion); err != nil {
 		log.Fatal(4, "InitIssuesIndexer: create index, %v", err)
 	}
 	if err = populateIndexer(); err != nil {
@@ -69,7 +69,7 @@ func InitIssueIndexer(populateIndexer func() error) {
 }
 
 // createIssueIndexer create an issue indexer if one does not already exist
-func createIssueIndexer() error {
+func createIssueIndexer(path string, latestVersion int) error {
 	mapping := bleve.NewIndexMapping()
 	docMapping := bleve.NewDocumentMapping()
 
@@ -100,8 +100,14 @@ func createIssueIndexer() error {
 	mapping.AddDocumentMapping("_all", bleve.NewDocumentDisabledMapping())
 
 	var err error
-	issueIndexer, err = bleve.New(setting.Indexer.IssuePath, mapping)
-	return err
+	issueIndexer, err = bleve.New(path, mapping)
+	if err != nil {
+		return err
+	}
+
+	return rupture.WriteIndexMetadata(path, &rupture.IndexMetadata{
+		Version: latestVersion,
+	})
 }
 
 // IssueIndexerBatch batch to add updates to

modules/indexer/repo.go

@@ -84,7 +84,7 @@ func InitRepoIndexer(populateIndexer func() error) {
 		return
 	}
 
-	if err = createRepoIndexer(); err != nil {
+	if err = createRepoIndexer(setting.Indexer.RepoPath, repoIndexerLatestVersion); err != nil {
 		log.Fatal(4, "CreateRepoIndexer: %v", err)
 	}
 	if err = populateIndexer(); err != nil {
@@ -93,7 +93,7 @@ func InitRepoIndexer(populateIndexer func() error) {
 }
 
 // createRepoIndexer create a repo indexer if one does not already exist
-func createRepoIndexer() error {
+func createRepoIndexer(path string, latestVersion int) error {
 	var err error
 	docMapping := bleve.NewDocumentMapping()
 	numericFieldMapping := bleve.NewNumericFieldMapping()
@@ -119,8 +119,13 @@ func createRepoIndexer() error {
 	mapping.AddDocumentMapping(repoIndexerDocType, docMapping)
 	mapping.AddDocumentMapping("_all", bleve.NewDocumentDisabledMapping())
 
-	repoIndexer, err = bleve.New(setting.Indexer.RepoPath, mapping)
-	return err
+	repoIndexer, err = bleve.New(path, mapping)
+	if err != nil {
+		return err
+	}
+	return rupture.WriteIndexMetadata(path, &rupture.IndexMetadata{
+		Version: latestVersion,
+	})
 }
 
 func filenameIndexerID(repoID int64, filename string) string {

public/js/index.js

@@ -1,5 +1,9 @@
 'use strict';
 
+function htmlEncode(text) {
+   return jQuery('<div />').text(text).html()
+}
+
 var csrf;
 var suburl;
 
@@ -312,12 +316,12 @@ function initCommentForm() {
             switch (input_id) {
                 case '#milestone_id':
                     $list.find('.selected').html('<a class="item" href=' + $(this).data('href') + '>' +
-                        $(this).text() + '</a>');
+                        htmlEncode($(this).text()) + '</a>');
                     break;
                 case '#assignee_id':
                     $list.find('.selected').html('<a class="item" href=' + $(this).data('href') + '>' +
                         '<img class="ui avatar image" src=' + $(this).data('avatar') + '>' +
-                        $(this).text() + '</a>');
+                        htmlEncode($(this).text()) + '</a>');
             }
             $('.ui' + select_id + '.list .no-select').addClass('hide');
             $(input_id).val($(this).data('id'));
@@ -1456,7 +1460,7 @@ function searchUsers() {
                 $.each(response.data, function (i, item) {
                     var title = item.login;
                     if (item.full_name && item.full_name.length > 0) {
-                        title += ' (' + item.full_name + ')';
+                        title += ' (' + htmlEncode(item.full_name) + ')';
                     }
                     items.push({
                         title: title,
@@ -2510,7 +2514,7 @@ function initTopicbar() {
                 if (res.topics) {
                     formattedResponse.success = true;
                     for (var i=0;i < res.topics.length;i++) {
-                        formattedResponse.results.push({"description": res.topics[i].Name, "data-value":res.topics[i].Name})
+                        formattedResponse.results.push({"description": res.topics[i].Name, "data-value": res.topics[i].Name})
                     }
                 }
 
@@ -2631,7 +2635,7 @@ function initIssueList() {
                     // Parse the response from the api to work with our dropdown
                     $.each(response, function(index, issue) {
                         filteredResponse.results.push({
-                            'name'  : '#' + issue.number + '&nbsp;' + issue.title,
+                            'name'  : '#' + issue.number + '&nbsp;' + htmlEncode(issue.title),
                             'value' : issue.id
                         });
                     });

routers/repo/editor.go

@@ -559,6 +559,17 @@ func UploadFilePost(ctx *context.Context, form auth.UploadRepoFileForm) {
 	ctx.Redirect(ctx.Repo.RepoLink + "/src/branch/" + branchName + "/" + form.TreePath)
 }
 
+func cleanUploadFileName(name string) string {
+	name = strings.TrimLeft(name, "./\\")
+	name = strings.Replace(name, "../", "", -1)
+	name = strings.Replace(name, "..\\", "", -1)
+	name = strings.TrimPrefix(path.Clean(name), ".git/")
+	if name == ".git" {
+		return ""
+	}
+	return name
+}
+
 // UploadFileToServer upload file to server file dir not git
 func UploadFileToServer(ctx *context.Context) {
 	file, header, err := ctx.Req.FormFile("file")
@@ -591,7 +602,13 @@ func UploadFileToServer(ctx *context.Context) {
 		}
 	}
 
-	upload, err := models.NewUpload(header.Filename, buf, file)
+	name := cleanUploadFileName(header.Filename)
+	if len(name) == 0 {
+		ctx.Error(500, "Upload file name is invalid")
+		return
+	}
+
+	upload, err := models.NewUpload(name, buf, file)
 	if err != nil {
 		ctx.Error(500, fmt.Sprintf("NewUpload: %v", err))
 		return

routers/repo/editor_test.go

@@ -0,0 +1,30 @@
+// Copyright 2018 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package repo
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCleanUploadName(t *testing.T) {
+	models.PrepareTestEnv(t)
+
+	var kases = map[string]string{
+		".git/refs/master": "git/refs/master",
+		"/root/abc":        "root/abc",
+		"./../../abc":      "abc",
+		"a/../.git":        "a/.git",
+		"a/../../../abc":   "a/abc",
+		"../../../acd":     "acd",
+		"../../.git/abc":   "git/abc",
+		"..\\..\\.git/abc": "git/abc",
+	}
+	for k, v := range kases {
+		assert.EqualValues(t, v, cleanUploadFileName(k))
+	}
+}

routers/repo/wiki.go

@@ -73,7 +73,6 @@ func findWikiRepoCommit(ctx *context.Context) (*git.Repository, *git.Commit, err
 
 	commit, err := wikiRepo.GetBranchCommit("master")
 	if err != nil {
-		ctx.ServerError("GetBranchCommit", err)
 		return wikiRepo, nil, err
 	}
 	return wikiRepo, commit, nil
@@ -111,6 +110,9 @@ func wikiContentsByName(ctx *context.Context, commit *git.Commit, wikiName strin
 func renderWikiPage(ctx *context.Context, isViewPage bool) (*git.Repository, *git.TreeEntry) {
 	wikiRepo, commit, err := findWikiRepoCommit(ctx)
 	if err != nil {
+		if !git.IsErrNotExist(err) {
+			ctx.ServerError("GetBranchCommit", err)
+		}
 		return nil, nil
 	}
 

templates/base/footer.tmpl

@@ -51,8 +51,6 @@
 {{end}}
 {{if .RequireTribute}}
 	<script src="{{AppSubUrl}}/vendor/plugins/tribute/tribute.min.js"></script>
-
-	{{if .Assignees}}
 	<script>
 		var issuesTribute = new Tribute({
 			values: [
@@ -73,7 +71,6 @@
 		})
 		issuesTribute.attach(document.getElementById('content'))
 	</script>
-	{{end}}
 	<script>
 		var emojiTribute = new Tribute({
 			collection: [{