Merge pull request from GHSA-mfqj-f22m-gv8j

Fix IDOR webhook get does not correctly authorize project
rundeck · Oct 1, 2021 · a3bdc06 · a3bdc06
2 parents f739c61 + 962b81a
commit a3bdc06
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 1 deletion.
diff --git a/grails-webhooks/grails-app/controllers/webhooks/WebhookController.groovy b/grails-webhooks/grails-app/controllers/webhooks/WebhookController.groovy
@@ -88,7 +88,11 @@ class WebhookController {
             sendJsonError("You do not have access to this resource")
             return
         }
-        render webhookService.getWebhookWithAuth(params.id) as JSON
+        def webhook = webhookService.getWebhookForProjectWithAuth(params.id, params.project)
+        if(!webhook){
+            return sendJsonError("Webhook not found", HttpServletResponse.SC_NOT_FOUND)
+        }
+        render webhook as JSON
     }
 
     def list() {

diff --git a/grails-webhooks/grails-app/services/webhooks/WebhookService.groovy b/grails-webhooks/grails-app/services/webhooks/WebhookService.groovy
@@ -290,6 +290,13 @@ class WebhookService {
         Webhook hook = Webhook.get(id.toLong())
         getWebhookWithAuthAsMap(hook)
     }
+    def getWebhookForProjectWithAuth(String id, String project) {
+        Webhook hook = getWebhookWithProject(id.toLong(), project)
+        if(!hook){
+            return null
+        }
+        getWebhookWithAuthAsMap(hook)
+    }
 
     private Map getWebhookWithAuthAsMap(Webhook hook) {
         AuthenticationToken authToken = rundeckAuthTokenManagerService.getToken(hook.authToken)
@@ -299,6 +306,9 @@ class WebhookService {
     Webhook getWebhook(Long id) {
         return Webhook.get(id)
     }
+    Webhook getWebhookWithProject(Long id, String project) {
+        return Webhook.findByIdAndProject(id,project)
+    }
 
     Webhook getWebhookByUuid(String uuid) {
         return Webhook.findByUuid(uuid)

diff --git a/grails-webhooks/src/test/groovy/webhooks/WebhookControllerSpec.groovy b/grails-webhooks/src/test/groovy/webhooks/WebhookControllerSpec.groovy
@@ -24,6 +24,7 @@ import com.dtolabs.rundeck.plugins.webhook.DefaultWebhookResponder
 import com.dtolabs.rundeck.plugins.webhook.WebhookDataImpl
 import com.dtolabs.rundeck.plugins.webhook.WebhookResponder
 import grails.testing.web.controllers.ControllerUnitTest
+import org.rundeck.core.auth.AuthConstants
 import spock.lang.Specification
 
 import javax.servlet.http.HttpServletRequest
@@ -96,6 +97,32 @@ class WebhookControllerSpec extends Specification implements ControllerUnitTest<
         0 * controller.webhookService.delete(_)
     }
 
+    def "get webhook should fail when project param does not match webhook project"() {
+        given:
+            controller.webhookService = Mock(WebhookService) {
+                1 * getWebhookForProjectWithAuth('1234','otherproject')
+            }
+
+            controller.apiService = Mock(MockApiService)
+            controller.rundeckAuthContextEvaluator = Mock(AuthContextEvaluator)
+            controller.rundeckAuthContextProvider = Mock(AuthContextProvider)
+        when:
+            params.id = "1234"
+            params.project = 'otherproject'
+            controller.get()
+
+        then:
+            response.status==404
+            response.text == '{"err":"Webhook not found"}'
+            1 * controller.rundeckAuthContextProvider.getAuthContextForSubjectAndProject(_,'otherproject')
+            1 * controller.rundeckAuthContextEvaluator.authorizeProjectResourceAny(
+                _,
+                AuthConstants.RESOURCE_TYPE_WEBHOOK,
+                [AuthConstants.ACTION_ADMIN, AuthConstants.ACTION_READ],
+                'otherproject'
+            ) >> true
+    }
+
     def "503 if webhook is not enabled"() {
         given:
         controller.webhookService = Mock(MockWebhookService)

diff --git a/grails-webhooks/src/test/groovy/webhooks/WebhookServiceSpec.groovy b/grails-webhooks/src/test/groovy/webhooks/WebhookServiceSpec.groovy
@@ -494,6 +494,27 @@ class WebhookServiceSpec extends Specification implements ServiceUnitTest<Webhoo
         output.authToken == "abc123"
     }
 
+    def "getWebhookForProjectWithAuth returns null for wrong project"() {
+        setup:
+            def project = 'aproject'
+            Webhook hook = new Webhook()
+            hook.name = "hit"
+            hook.project = "otherproject"
+            hook.authToken = "abc123"
+            hook.eventPlugin = "do-some-action"
+            hook.pluginConfigurationJson = '{"prop1":"true"}'
+            hook.save()
+
+            service.rundeckAuthTokenManagerService = Mock(AuthTokenManager)
+
+        when:
+            def output = service.getWebhookForProjectWithAuth(hook.id.toString(), project)
+
+        then:
+            output == null
+            0 * service.rundeckAuthTokenManagerService.getToken("abc123")
+    }
+
     def "delete all webhooks in project"() {
         setup:
         String project = "prj1"