CVE-2023-40034: Insufficient Information (NVD-CWE-noinfo)
value: Woodpecker is a community fork of the Drone CI system. In affected versions an attacker can post malformed webhook data witch lead to an update of the repository data that can e.g. allow the takeover of an repo. This is only critical if the CI is configured for public usage and connected to a forge witch is also in public usage. This issue has been addressed in version 1.0.2. Users are advised to upgrade. Users unable to upgrade should secure the CI system by making it inaccessible to untrusted entities, for example, by placing it behind a firewall.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
CVSS v3 base score: 8.1
Published: 2023-08-16T21:15Z
Repository: https://github.com/woodpecker-ci/woodpecker
Language: Go
Files changed: server/api/hook.go
Commit date: 2023-08-16 16:19:10 +0200
Commit message: Validate webhook before change any data (#2221) (#2222)

Backport #2221
Diff: diff --git a/server/api/hook.go b/server/api/hook.go
index 3fca7296..74e0ac65 100644
--- a/server/api/hook.go
+++ b/server/api/hook.go
@@ -106,10 +106,14 @@ func BlockTilQueueHasRunningItem(c *gin.Context) {
 //	@Param		hook	body	object	true	"the webhook payload; forge is automatically detected"
 func PostHook(c *gin.Context) {
 	_store := store.FromContext(c)
 	forge := server.Config.Services.Forge
 
+	//
+	// 1. Parse webhook
+	//
+
 	tmpRepo, tmpBuild, err := forge.Hook(c, c.Request)
 	if err != nil {
 		if errors.Is(err, &types.ErrIgnoreEvent{}) {
 			msg := fmt.Sprintf("forge driver: %s", err)
 			log.Debug().Err(err).Msg(msg)
@@ -134,20 +138,29 @@ func PostHook(c *gin.Context) {
 		log.Debug().Msg(msg)
 		c.String(http.StatusBadRequest, msg)
 		return
 	}
 
+	//
+	// Skip if commit message contains skip-ci
+	// TODO: move into global pipeline conditions logic
+	//
+
 	// skip the tmpBuild if any case-insensitive combination of the words "skip" and "ci"
 	// wrapped in square brackets appear in the commit message
 	skipMatch := skipRe.FindString(tmpBuild.Message)
 	if len(skipMatch) > 0 {
 		msg := fmt.Sprintf("ignoring hook: %s found in %s", skipMatch, tmpBuild.Commit)
 		log.Debug().Msg(msg)
 		c.String(http.StatusNoContent, msg)
 		return
 	}
 
+	//
+	// 2. Get related repo from store and take repo renaming into account
+	//
+
 	repo, err := _store.GetRepoNameFallback(tmpRepo.ForgeRemoteID, tmpRepo.FullName)
 	if err != nil {
 		msg := fmt.Sprintf("failure to get repo %s from store", tmpRepo.FullName)
 		log.Error().Err(err).Msg(msg)
 		c.String(http.StatusNotFound, msg)
@@ -157,28 +170,23 @@ func PostHook(c *gin.Context) {
 		msg := fmt.Sprintf("ignoring hook: repo %s is inactive", tmpRepo.FullName)
 		log.Debug().Msg(msg)
 		c.String(http.StatusNoContent, msg)
 		return
 	}
-
 	oldFullName := repo.FullName
-	if oldFullName != tmpRepo.FullName {
-		// create a redirection
-		err = _store.CreateRedirection(&model.Redirection{RepoID: repo.ID, FullName: repo.FullName})
-		if err != nil {
-			_ = c.AbortWithError(http.StatusInternalServerError, err)
-			return
-		}
-	}
 
-	repo.Update(tmpRepo)
-	err = _store.UpdateRepo(repo)
-	if err != nil {
-		c.String(http.StatusInternalServerError, err.Error())
+	if repo.UserID == 0 {
+		msg := fmt.Sprintf("ignoring hook. repo %s has no owner.", repo.FullName)
+		log.Warn().Msg(msg)
+		c.String(http.StatusNoContent, msg)
 		return
 	}
 
+	//
+	// 3. Check if the webhook is a valid and authorized one
+	//
+
 	// get the token and verify the hook is authorized
 	parsed, err := token.ParseRequest(c.Request, func(_ *token.Token) (string, error) {
 		return repo.Hash, nil
 	})
 	if err != nil {
@@ -203,24 +211,45 @@ func PostHook(c *gin.Context) {
 		log.Debug().Msg(msg)
 		c.String(http.StatusForbidden, msg)
 		return
 	}
 
-	if repo.UserID == 0 {
-		msg := fmt.Sprintf("ignoring hook. repo %s has no owner.", repo.FullName)
-		log.Warn().Msg(msg)
-		c.String(http.StatusNoContent, msg)
+	//
+	// 4. Update repo
+	//
+
+	if oldFullName != tmpRepo.FullName {
+		// create a redirection
+		err = _store.CreateRedirection(&model.Redirection{RepoID: repo.ID, FullName: repo.FullName})
+		if err != nil {
+			_ = c.AbortWithError(http.StatusInternalServerError, err)
+			return
+		}
+	}
+
+	repo.Update(tmpRepo)
+	err = _store.UpdateRepo(repo)
+	if err != nil {
+		c.String(http.StatusInternalServerError, err.Error())
 		return
 	}
 
+	//
+	// 5. Check if pull requests are allowed for this repo
+	//
+
 	if tmpBuild.Event == model.EventPull && !repo.AllowPull {
 		msg := "ignoring hook: pull requests are disabled for this repo in woodpecker"
 		log.Debug().Str("repo", repo.FullName).Msg(msg)
 		c.String(http.StatusNoContent, msg)
 		return
 	}
 
+	//
+	// 6. Finally create a pipeline
+	//
+
 	pl, err := pipeline.Create(c, _store, repo, tmpBuild)
 	if err != nil {
 		handlePipelineErr(c, err)
 	} else {
 		c.JSON(http.StatusOK, pl)

Vulnerable code: 
	if oldFullName != tmpRepo.FullName {
		// create a redirection
		err = _store.CreateRedirection(&model.Redirection{RepoID: repo.ID, FullName: repo.FullName})
		if err != nil {
			_ = c.AbortWithError(http.StatusInternalServerError, err)
			return
		}
	}
	repo.Update(tmpRepo)
	err = _store.UpdateRepo(repo)
	if err != nil {
		c.String(http.StatusInternalServerError, err.Error())
	if repo.UserID == 0 {
		msg := fmt.Sprintf("ignoring hook. repo %s has no owner.", repo.FullName)
		log.Warn().Msg(msg)
		c.String(http.StatusNoContent, msg)
Fixed code: 	//
	// 1. Parse webhook
	//

	//
	// Skip if commit message contains skip-ci
	// TODO: move into global pipeline conditions logic
	//

	//
	// 2. Get related repo from store and take repo renaming into account
	//

	if repo.UserID == 0 {
		msg := fmt.Sprintf("ignoring hook. repo %s has no owner.", repo.FullName)
		log.Warn().Msg(msg)
		c.String(http.StatusNoContent, msg)
	//
	// 3. Check if the webhook is a valid and authorized one
	//

	//
	// 4. Update repo
	//

	if oldFullName != tmpRepo.FullName {
		// create a redirection
		err = _store.CreateRedirection(&model.Redirection{RepoID: repo.ID, FullName: repo.FullName})
		if err != nil {
			_ = c.AbortWithError(http.StatusInternalServerError, err)
			return
		}
	}

	repo.Update(tmpRepo)
	err = _store.UpdateRepo(repo)
	if err != nil {
		c.String(http.StatusInternalServerError, err.Error())
	//
	// 5. Check if pull requests are allowed for this repo
	//

	//
	// 6. Finally create a pipeline
	//