CVE-2023-28609: Improper Authentication (CWE-287)
value: api/auth.go in Ansible Semaphore before 2.8.89 mishandles authentication.
CWE description: When an actor claims to have a given identity, the product does not prove or insufficiently proves that the claim is correct.
CVSS v3 base score: 9.8
Published: 2023-03-18T21:15Z
Repository: https://github.com/ansible-semaphore/semaphore
Language: Go
Files changed: api/auth.go, api/router.go
Commit date: 2023-03-13 14:04:58 +0100
Commit message: fix: authentization bug

Diff: diff --git a/api/auth.go b/api/auth.go
index a69e581..4a70821 100644
--- a/api/auth.go
+++ b/api/auth.go
@@ -9,11 +9,11 @@ import (
 	"net/http"
 	"strings"
 	"time"
 )
 
-func authenticationHandler(w http.ResponseWriter, r *http.Request) {
+func authenticationHandler(w http.ResponseWriter, r *http.Request) bool {
 	var userID int
 
 	authHeader := strings.ToLower(r.Header.Get("authorization"))
 
 	if len(authHeader) > 0 && strings.Contains(authHeader, "bearer") {
@@ -23,44 +23,44 @@ func authenticationHandler(w http.ResponseWriter, r *http.Request) {
 			if err != db.ErrNotFound {
 				log.Error(err)
 			}
 
 			w.WriteHeader(http.StatusUnauthorized)
-			return
+			return false
 		}
 
 		userID = token.UserID
 	} else {
 		// fetch session from cookie
 		cookie, err := r.Cookie("semaphore")
 		if err != nil {
 			w.WriteHeader(http.StatusUnauthorized)
-			return
+			return false
 		}
 
 		value := make(map[string]interface{})
 		if err = util.Cookie.Decode("semaphore", cookie.Value, &value); err != nil {
 			w.WriteHeader(http.StatusUnauthorized)
-			return
+			return false
 		}
 
 		user, ok := value["user"]
 		sessionVal, okSession := value["session"]
 		if !ok || !okSession {
 			w.WriteHeader(http.StatusUnauthorized)
-			return
+			return false
 		}
 
 		userID = user.(int)
 		sessionID := sessionVal.(int)
 
 		// fetch session
 		session, err := helpers.Store(r).GetSession(userID, sessionID)
 
 		if err != nil {
 			w.WriteHeader(http.StatusUnauthorized)
-			return
+			return false
 		}
 
 		if time.Since(session.LastActive).Hours() > 7*24 {
 			// more than week old unused session
 			// destroy.
@@ -68,57 +68,64 @@ func authenticationHandler(w http.ResponseWriter, r *http.Request) {
 				// it is internal error, it doesn't concern the user
 				log.Error(err)
 			}
 
 			w.WriteHeader(http.StatusUnauthorized)
-			return
+			return false
 		}
 
 		if err := helpers.Store(r).TouchSession(userID, sessionID); err != nil {
 			log.Error(err)
 			w.WriteHeader(http.StatusUnauthorized)
-			return
+			return false
 		}
 	}
 
 	user, err := helpers.Store(r).GetUser(userID)
 	if err != nil {
 		if err != db.ErrNotFound {
 			// internal error
 			log.Error(err)
 		}
 		w.WriteHeader(http.StatusUnauthorized)
-		return
+		return false
 	}
 
 	if util.Config.DemoMode {
 		if !user.Admin && r.Method != "GET" &&
 			!strings.HasSuffix(r.URL.Path, "/tasks") &&
 			!strings.HasSuffix(r.URL.Path, "/stop") {
 			w.WriteHeader(http.StatusUnauthorized)
-			return
+			return false
 		}
 	}
 
 	context.Set(r, "user", &user)
+	return true
 }
 
 // nolint: gocyclo
 func authentication(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		authenticationHandler(w, r)
-		next.ServeHTTP(w, r)
+		ok := authenticationHandler(w, r)
+		if ok {
+			next.ServeHTTP(w, r)
+		}
 	})
 }
 
 // nolint: gocyclo
 func authenticationWithStore(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		store := helpers.Store(r)
 
+		var ok bool
+		
 		db.StoreSession(store, r.URL.String(), func() {
-			authenticationHandler(w, r)
+			ok = authenticationHandler(w, r)
 		})
 
-		next.ServeHTTP(w, r)
+		if ok {
+			next.ServeHTTP(w, r)
+		}
 	})
 }
diff --git a/api/router.go b/api/router.go
index b5a3d8e..1a51c31 100644
--- a/api/router.go
+++ b/api/router.go
@@ -346,20 +346,12 @@ func servePublic(w http.ResponseWriter, r *http.Request) {
 	_, err = w.Write(res)
 	util.LogWarning(err)
 }
 
 func getSystemInfo(w http.ResponseWriter, r *http.Request) {
-	//updateAvailable, err := util.CheckUpdate()
-
-	//if err != nil {
-	//	helpers.WriteError(w, err)
-	//	return
-	//}
-
 	body := map[string]interface{}{
 		"version": util.Version,
-		//"update":  updateAvailable,
 		"ansible": util.AnsibleVersion(),
 		"demo":    util.Config.DemoMode,
 	}
 
 	helpers.WriteJSON(w, http.StatusOK, body)

Vulnerable code: func authenticationHandler(w http.ResponseWriter, r *http.Request) {
			return
			return
			return
			return
			return
			return
			return
		return
			return
		authenticationHandler(w, r)
		next.ServeHTTP(w, r)
			authenticationHandler(w, r)
		next.ServeHTTP(w, r)
	//updateAvailable, err := util.CheckUpdate()

	//if err != nil {
	//	helpers.WriteError(w, err)
	//	return
	//}

		//"update":  updateAvailable,
Fixed code: func authenticationHandler(w http.ResponseWriter, r *http.Request) bool {
			return false
			return false
			return false
			return false
			return false
			return false
			return false
		return false
			return false
	return true
		ok := authenticationHandler(w, r)
		if ok {
			next.ServeHTTP(w, r)
		}
		var ok bool
		
			ok = authenticationHandler(w, r)
		if ok {
			next.ServeHTTP(w, r)
		}