From a9406764ee06b696d4867045bd951bfb1b45c1a1 Mon Sep 17 00:00:00 2001
From: Hao Zhang <hao.zhang@portainer.io>
Date: Thu, 27 Jan 2022 08:38:29 +0800
Subject: [PATCH] fix(service): webhook vulnerability for passing an invalid
 image tag EE-2121 (#6269)

* fix(service): webhook vulnerability for passing an invalid image tag
---
 api/http/handler/webhooks/handler.go          | 48 ++++++++++++++++++-
 api/http/handler/webhooks/webhook_create.go   |  9 ++++
 api/http/handler/webhooks/webhook_delete.go   | 11 +++++
 api/http/handler/webhooks/webhook_execute.go  | 11 ++++-
 api/http/handler/webhooks/webhook_list.go     |  9 ++++
 api/http/handler/webhooks/webhook_update.go   | 10 ++++
 api/internal/authorization/authorizations.go  | 18 +++++++
 .../views/services/create/createservice.html  |  2 +-
 app/docker/views/services/edit/service.html   |  2 +-
 9 files changed, 115 insertions(+), 5 deletions(-)

diff --git a/api/http/handler/webhooks/handler.go b/api/http/handler/webhooks/handler.go
index b0136d0ef..665b70365 100644
--- a/api/http/handler/webhooks/handler.go
+++ b/api/http/handler/webhooks/handler.go
@@ -1,11 +1,13 @@
 package webhooks
 
 import (
+	"github.com/portainer/portainer/api/dataservices"
+	"github.com/portainer/portainer/api/internal/authorization"
 	"net/http"
 
 	"github.com/gorilla/mux"
 	httperror "github.com/portainer/libhttp/error"
-	"github.com/portainer/portainer/api/dataservices"
+	portainer "github.com/portainer/portainer/api"
 	"github.com/portainer/portainer/api/docker"
 	"github.com/portainer/portainer/api/http/security"
 )
@@ -13,6 +15,7 @@ import (
 // Handler is the HTTP handler used to handle webhook operations.
 type Handler struct {
 	*mux.Router
+	requestBouncer      *security.RequestBouncer
 	DataStore           dataservices.DataStore
 	DockerClientFactory *docker.ClientFactory
 }
@@ -20,7 +23,8 @@ type Handler struct {
 // NewHandler creates a handler to manage webhooks operations.
 func NewHandler(bouncer *security.RequestBouncer) *Handler {
 	h := &Handler{
-		Router: mux.NewRouter(),
+		Router:         mux.NewRouter(),
+		requestBouncer: bouncer,
 	}
 	h.Handle("/webhooks",
 		bouncer.AuthenticatedAccess(httperror.LoggerHandler(h.webhookCreate))).Methods(http.MethodPost)
@@ -34,3 +38,43 @@ func NewHandler(bouncer *security.RequestBouncer) *Handler {
 		bouncer.PublicAccess(httperror.LoggerHandler(h.webhookExecute))).Methods(http.MethodPost)
 	return h
 }
+
+func (handler *Handler) checkResourceAccess(r *http.Request, resourceID string, resourceControlType portainer.ResourceControlType) *httperror.HandlerError {
+	securityContext, err := security.RetrieveRestrictedRequestContext(r)
+	if err != nil {
+		return &httperror.HandlerError{StatusCode: http.StatusInternalServerError, Message: "Unable to retrieve user info from request context", Err: err}
+	}
+	// non-admins
+	rc, err := handler.DataStore.ResourceControl().ResourceControlByResourceIDAndType(resourceID, resourceControlType)
+	if rc == nil || err != nil {
+		return &httperror.HandlerError{StatusCode: http.StatusInternalServerError, Message: "Unable to retrieve a resource control associated to the resource", Err: err}
+	}
+	userTeamIDs := make([]portainer.TeamID, 0)
+	for _, membership := range securityContext.UserMemberships {
+		userTeamIDs = append(userTeamIDs, membership.TeamID)
+	}
+	canAccess := authorization.UserCanAccessResource(securityContext.UserID, userTeamIDs, rc)
+	if !canAccess {
+		return &httperror.HandlerError{StatusCode: http.StatusForbidden, Message: "This operation is disabled for non-admin users and unassigned access users"}
+	}
+	return nil
+}
+
+func (handler *Handler) checkAuthorization(r *http.Request, endpoint *portainer.Endpoint, authorizations []portainer.Authorization) (bool, *httperror.HandlerError) {
+	err := handler.requestBouncer.AuthorizedEndpointOperation(r, endpoint)
+	if err != nil {
+		return false, &httperror.HandlerError{StatusCode: http.StatusForbidden, Message: "Permission denied to access environment", Err: err}
+	}
+
+	securityContext, err := security.RetrieveRestrictedRequestContext(r)
+	if err != nil {
+		return false, &httperror.HandlerError{StatusCode: http.StatusInternalServerError, Message: "Unable to retrieve user info from request context", Err: err}
+	}
+
+	authService := authorization.NewService(handler.DataStore)
+	isAdminOrAuthorized, err := authService.UserIsAdminOrAuthorized(securityContext.UserID, endpoint.ID, authorizations)
+	if err != nil {
+		return false, &httperror.HandlerError{StatusCode: http.StatusInternalServerError, Message: "Unable to get user authorizations", Err: err}
+	}
+	return isAdminOrAuthorized, nil
+}
diff --git a/api/http/handler/webhooks/webhook_create.go b/api/http/handler/webhooks/webhook_create.go
index 78326b720..b0d7b00dc 100644
--- a/api/http/handler/webhooks/webhook_create.go
+++ b/api/http/handler/webhooks/webhook_create.go
@@ -64,6 +64,15 @@ func (handler *Handler) webhookCreate(w http.ResponseWriter, r *http.Request) *h
 
 	endpointID := portainer.EndpointID(payload.EndpointID)
 
+	securityContext, err := security.RetrieveRestrictedRequestContext(r)
+	if err != nil {
+		return &httperror.HandlerError{StatusCode: http.StatusInternalServerError, Message: "Unable to retrieve user info from request context", Err: err}
+	}
+
+	if !securityContext.IsAdmin {
+		return &httperror.HandlerError{StatusCode: http.StatusForbidden, Message: "Not authorized to create a webhook", Err: errors.New("not authorized to create a webhook")}
+	}
+
 	if payload.RegistryID != 0 {
 		tokenData, err := security.RetrieveTokenData(r)
 		if err != nil {
diff --git a/api/http/handler/webhooks/webhook_delete.go b/api/http/handler/webhooks/webhook_delete.go
index bd94ce9a5..422316c15 100644
--- a/api/http/handler/webhooks/webhook_delete.go
+++ b/api/http/handler/webhooks/webhook_delete.go
@@ -1,6 +1,8 @@
 package webhooks
 
 import (
+	"errors"
+	"github.com/portainer/portainer/api/http/security"
 	"net/http"
 
 	httperror "github.com/portainer/libhttp/error"
@@ -25,6 +27,15 @@ func (handler *Handler) webhookDelete(w http.ResponseWriter, r *http.Request) *h
 		return &httperror.HandlerError{http.StatusBadRequest, "Invalid webhook id", err}
 	}
 
+	securityContext, err := security.RetrieveRestrictedRequestContext(r)
+	if err != nil {
+		return &httperror.HandlerError{StatusCode: http.StatusInternalServerError, Message: "Unable to retrieve user info from request context", Err: err}
+	}
+
+	if !securityContext.IsAdmin {
+		return &httperror.HandlerError{StatusCode: http.StatusForbidden, Message: "Not authorized to delete a webhook", Err: errors.New("not authorized to delete a webhook")}
+	}
+
 	err = handler.DataStore.Webhook().DeleteWebhook(portainer.WebhookID(id))
 	if err != nil {
 		return &httperror.HandlerError{http.StatusInternalServerError, "Unable to remove the webhook from the database", err}
diff --git a/api/http/handler/webhooks/webhook_execute.go b/api/http/handler/webhooks/webhook_execute.go
index cff0b3305..9b0d581d7 100644
--- a/api/http/handler/webhooks/webhook_execute.go
+++ b/api/http/handler/webhooks/webhook_execute.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"github.com/portainer/portainer/api/internal/registryutils"
+	"io"
 	"net/http"
 	"strings"
 
@@ -111,7 +112,15 @@ func (handler *Handler) executeServiceWebhook(
 			}
 		}
 	}
-
+	if imageTag != "" {
+		rc, err := dockerClient.ImagePull(context.Background(), service.Spec.TaskTemplate.ContainerSpec.Image, dockertypes.ImagePullOptions{RegistryAuth: serviceUpdateOptions.EncodedRegistryAuth})
+		if err != nil {
+			return &httperror.HandlerError{StatusCode: http.StatusNotFound, Message: "Error pulling image with the specified tag", Err: err}
+		}
+		defer func(rc io.ReadCloser) {
+			_ = rc.Close()
+		}(rc)
+	}
 	_, err = dockerClient.ServiceUpdate(context.Background(), resourceID, service.Version, service.Spec, serviceUpdateOptions)
 
 	if err != nil {
diff --git a/api/http/handler/webhooks/webhook_list.go b/api/http/handler/webhooks/webhook_list.go
index a6f26889d..46ef0a18d 100644
--- a/api/http/handler/webhooks/webhook_list.go
+++ b/api/http/handler/webhooks/webhook_list.go
@@ -1,6 +1,7 @@
 package webhooks
 
 import (
+	"github.com/portainer/portainer/api/http/security"
 	"net/http"
 
 	httperror "github.com/portainer/libhttp/error"
@@ -33,6 +34,14 @@ func (handler *Handler) webhookList(w http.ResponseWriter, r *http.Request) *htt
 		return &httperror.HandlerError{http.StatusBadRequest, "Invalid query parameter: filters", err}
 	}
 
+	securityContext, err := security.RetrieveRestrictedRequestContext(r)
+	if err != nil {
+		return &httperror.HandlerError{StatusCode: http.StatusInternalServerError, Message: "Unable to retrieve user info from request context", Err: err}
+	}
+	if !securityContext.IsAdmin {
+		return response.JSON(w, []portainer.Webhook{})
+	}
+
 	webhooks, err := handler.DataStore.Webhook().Webhooks()
 	webhooks = filterWebhooks(webhooks, &filters)
 	if err != nil {
diff --git a/api/http/handler/webhooks/webhook_update.go b/api/http/handler/webhooks/webhook_update.go
index 2f51abb15..0fff1ec82 100644
--- a/api/http/handler/webhooks/webhook_update.go
+++ b/api/http/handler/webhooks/webhook_update.go
@@ -1,6 +1,7 @@
 package webhooks
 
 import (
+	"errors"
 	"net/http"
 
 	"github.com/portainer/portainer/api/http/security"
@@ -53,6 +54,15 @@ func (handler *Handler) webhookUpdate(w http.ResponseWriter, r *http.Request) *h
 		return &httperror.HandlerError{http.StatusInternalServerError, "Unable to find a webhooks with the specified identifier inside the database", err}
 	}
 
+	securityContext, err := security.RetrieveRestrictedRequestContext(r)
+	if err != nil {
+		return &httperror.HandlerError{StatusCode: http.StatusInternalServerError, Message: "Unable to retrieve user info from request context", Err: err}
+	}
+
+	if !securityContext.IsAdmin {
+		return &httperror.HandlerError{StatusCode: http.StatusForbidden, Message: "Not authorized to update a webhook", Err: errors.New("not authorized to update a webhook")}
+	}
+
 	if payload.RegistryID != 0 {
 		tokenData, err := security.RetrieveTokenData(r)
 		if err != nil {
diff --git a/api/internal/authorization/authorizations.go b/api/internal/authorization/authorizations.go
index 6557e4d85..df3185b73 100644
--- a/api/internal/authorization/authorizations.go
+++ b/api/internal/authorization/authorizations.go
@@ -603,3 +603,21 @@ func getAuthorizationsFromRoles(roleIdentifiers []portainer.RoleID, roles []port
 
 	return authorizations
 }
+
+func (service *Service) UserIsAdminOrAuthorized(userID portainer.UserID, endpointID portainer.EndpointID, authorizations []portainer.Authorization) (bool, error) {
+	user, err := service.dataStore.User().User(userID)
+	if err != nil {
+		return false, err
+	}
+	if user.Role == portainer.AdministratorRole {
+		return true, nil
+	}
+
+	for _, authorization := range authorizations {
+		_, authorized := user.EndpointAuthorizations[endpointID][authorization]
+		if authorized {
+			return true, nil
+		}
+	}
+	return false, nil
+}
diff --git a/app/docker/views/services/create/createservice.html b/app/docker/views/services/create/createservice.html
index a491209ea..e1f05185f 100644
--- a/app/docker/views/services/create/createservice.html
+++ b/app/docker/views/services/create/createservice.html
@@ -95,7 +95,7 @@
           </div>
           <!-- !port-mapping -->
           <!-- create-webhook -->
-          <div ng-if="endpoint.Type !== 4">
+          <div ng-if="endpoint.Type !== 4 && isAdmin">
             <div class="col-sm-12 form-section-title"> Webhooks </div>
             <div class="form-group">
               <div class="col-sm-12">
diff --git a/app/docker/views/services/edit/service.html b/app/docker/views/services/edit/service.html
index 3b1de5a71..41cc84db4 100644
--- a/app/docker/views/services/edit/service.html
+++ b/app/docker/views/services/edit/service.html
@@ -74,7 +74,7 @@
               <td>Image</td>
               <td>{{ service.Image }}</td>
             </tr>
-            <tr ng-if="applicationState.endpoint.type !== 4">
+            <tr ng-if="isAdmin && applicationState.endpoint.type !== 4">
               <td colspan="{{ webhookURL ? '1' : '2' }}">
                 Service webhook
                 <portainer-tooltip