From dfeefe0caabc46a1676b042b9ea189fc8892e4c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alvaro=20Mu=C3=B1oz?= Date: Thu, 16 May 2024 16:17:26 +0200 Subject: [PATCH] Consider actor and association checks as bypassable checks ONLY for issueOps --- ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUCritical.ql | 4 +++- ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUHigh.ql | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUCritical.ql b/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUCritical.ql index 5d501f2cea9..6b3e0628f40 100644 --- a/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUCritical.ql +++ b/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUCritical.ql @@ -15,15 +15,17 @@ import actions import codeql.actions.security.UntrustedCheckoutQuery import codeql.actions.security.PoisonableSteps -from LabelControlCheck check, MutableRefCheckoutStep checkout +from ControlCheck check, MutableRefCheckoutStep checkout where // the mutable checkout step is protected by an access check check = [checkout.getIf(), checkout.getEnclosingJob().getIf()] and // the checked-out code may lead to arbitrary code execution checkout.getAFollowingStep() instanceof PoisonableStep and ( + // label gates do not depend on the triggering event check instanceof LabelControlCheck or + // actor or Association gates apply to IssueOps only (check instanceof AssociationControlCheck or check instanceof ActorControlCheck) and check.getEnclosingJob().getATriggerEvent().getName().matches("%_comment") ) diff --git a/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUHigh.ql b/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUHigh.ql index e2f2b26a75c..fcf83269960 100644 --- a/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUHigh.ql +++ b/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUHigh.ql @@ -15,15 +15,17 @@ import actions import codeql.actions.security.UntrustedCheckoutQuery import codeql.actions.security.PoisonableSteps -from LabelControlCheck check, MutableRefCheckoutStep checkout +from ControlCheck check, MutableRefCheckoutStep checkout where // the mutable checkout step is protected by an access check check = [checkout.getIf(), checkout.getEnclosingJob().getIf()] and // there are no evidences that the checked-out code can lead to arbitrary code execution not checkout.getAFollowingStep() instanceof PoisonableStep and ( + // label gates do not depend on the triggering event check instanceof LabelControlCheck or + // actor or Association gates apply to IssueOps only (check instanceof AssociationControlCheck or check instanceof ActorControlCheck) and check.getEnclosingJob().getATriggerEvent().getName().matches("%_comment") )