From 8ed7d3da8862695b18aee25325eb00234995722d Mon Sep 17 00:00:00 2001 From: Kylie Stradley <4666485+KyFaSt@users.noreply.github.com> Date: Thu, 16 Jan 2025 18:03:28 +0000 Subject: [PATCH 1/2] Add Minimum Permissions Map * detect commonly used GitHub owned actions use a map to suggest the minimum permissions needed for the GITHUB_TOKEN --- .../ql/lib/codeql/actions/config/Config.qll | 12 +++++ .../actions/config/ConfigExtensions.qll | 5 +++ .../security/MinimumActionsPermissions.qll | 14 ++++++ .../ext/config/minimum_permissions_map.yml | 25 +++++++++++ .../CWE-275/MissingActionsPermissions.ql | 45 ++++++++++++++----- 5 files changed, 90 insertions(+), 11 deletions(-) create mode 100644 actions/ql/lib/codeql/actions/security/MinimumActionsPermissions.qll create mode 100644 actions/ql/lib/ext/config/minimum_permissions_map.yml diff --git a/actions/ql/lib/codeql/actions/config/Config.qll b/actions/ql/lib/codeql/actions/config/Config.qll index 265d4bd820f8..ca16e9d3b6d5 100644 --- a/actions/ql/lib/codeql/actions/config/Config.qll +++ b/actions/ql/lib/codeql/actions/config/Config.qll @@ -126,6 +126,18 @@ predicate vulnerableActionsDataModel( */ predicate immutableActionsDataModel(string action) { Extensions::immutableActionsDataModel(action) } +/** + * MaD models for minimum permissions for actions + * Fields: + * - action: action name + * - minimum_permissions: list of minimum permissions + */ +predicate minimumPermissionsDataModel( + string action, string minimum_permissions +) { + Extensions::minimumPermissionsDataModel(action, minimum_permissions) +} + /** * MaD models for untrusted git commands * Fields: diff --git a/actions/ql/lib/codeql/actions/config/ConfigExtensions.qll b/actions/ql/lib/codeql/actions/config/ConfigExtensions.qll index 99ad7eb8df1b..7f909adf7f93 100644 --- a/actions/ql/lib/codeql/actions/config/ConfigExtensions.qll +++ b/actions/ql/lib/codeql/actions/config/ConfigExtensions.qll @@ -63,6 +63,11 @@ extensible predicate vulnerableActionsDataModel( */ extensible predicate immutableActionsDataModel(string action); +/** + * Holds for actions that have a minimum permissions definition. + */ +extensible predicate minimumPermissionsDataModel(string action, string minimum_permissions); + /** * Holds for git commands that may introduce untrusted data when called on an attacker controlled branch. */ diff --git a/actions/ql/lib/codeql/actions/security/MinimumActionsPermissions.qll b/actions/ql/lib/codeql/actions/security/MinimumActionsPermissions.qll new file mode 100644 index 000000000000..a11371a93d8c --- /dev/null +++ b/actions/ql/lib/codeql/actions/security/MinimumActionsPermissions.qll @@ -0,0 +1,14 @@ +import actions +class MinimumActionsPermissions extends UsesStep { + string action; + string minimum_permissions; + + MinimumActionsPermissions() { + minimumPermissionsDataModel(action, minimum_permissions) and + this.getCallee() = action + } + + string getMinimumPermissions() { result = minimum_permissions } + + string getAction() { result = action } +} diff --git a/actions/ql/lib/ext/config/minimum_permissions_map.yml b/actions/ql/lib/ext/config/minimum_permissions_map.yml new file mode 100644 index 000000000000..10c8f827c237 --- /dev/null +++ b/actions/ql/lib/ext/config/minimum_permissions_map.yml @@ -0,0 +1,25 @@ +: + - addsTo: + pack: github/actions-all + extensible: minimumPermissionsDataModel + data: + - ["actions/cache", "{}"] + - ["actions/setup-node", "contents:read"] + - ["actions/upload-artifact", "{}"] + - ["actions/setup-python", "contents:read"] + - ["actions/download-artifact", "{}"] + - ["actions/github-script", "It depends on what the script does"] + - ["actions/setup-java", "contents:read"] + - ["actions/setup-go", "contents:read"] + - ["actions/setup-dotnet", "contents:read"] + - ["actions/labeler", "contents:read, pull-requests:write"] + - ["actions/attest", "id-token:write, attestations:write"] + - ["actions/add-to-project", "repository-projects:read, repository-projects:write, issues:read, pull-requests:read"] + - ["actions/dependency-review-action", "contents:read"] + - ["actions/attest-sbom", "id-token:write, attestations:write"] + - ["actions/stale", "contents:write, issues:write, pull-requests:write"] + - ["actions/attest-build-provenance", "id-token:write, attestations:write"] + - ["actions/jekyll-build-pages", "contents:read, pages:write, id-token:write"] + - ["actions/publish-action", "contents:write"] + - ["actions/version-package-tools", "contents:read, actions:read"] + - ["actions/reusable-workflows", "contents:read, actions:read"] \ No newline at end of file diff --git a/actions/ql/src/Security/CWE-275/MissingActionsPermissions.ql b/actions/ql/src/Security/CWE-275/MissingActionsPermissions.ql index d2969b7d6e72..acd160fe637b 100644 --- a/actions/ql/src/Security/CWE-275/MissingActionsPermissions.ql +++ b/actions/ql/src/Security/CWE-275/MissingActionsPermissions.ql @@ -11,15 +11,38 @@ * external/cwe/cwe-275 */ -import actions + import actions + import codeql.actions.security.MinimumActionsPermissions -from Job job -where - not exists(job.getPermissions()) and - not exists(job.getEnclosingWorkflow().getPermissions()) and - // exists a trigger event that is not a workflow_call - exists(Event e | - e = job.getATriggerEvent() and - not e.getName() = "workflow_call" - ) -select job, "Actions Job or Workflow does not set permissions" + // Returns the minimum permissions for all of the uses steps + // that are children of the job separated by a comma + // e.g. "contents: read, packages: write". If we cannot determine + // the permission we fallback to "unknown" + string getMinPermissions(Job job) { + if unknownPermissions(job) = true then result = "unknown" else + result = minPermissions(job) + } + + string minPermissions(Job job) { + result = concat(job.getAChildNode*().(MinimumActionsPermissions).getMinimumPermissions(), ", ") + } + + // Holds if we cannot determine the permissions for the uses step + // using the data extension or there are no uses steps + // that are children of the job + boolean unknownPermissions(Job job) { + minPermissions(job) = "" and result = true or count(job.getAChildNode*().(MinimumActionsPermissions)) = 0 and result = true + } + + from Job job + where + not exists(job.getPermissions()) and + not exists(job.getEnclosingWorkflow().getPermissions()) and + // exists a trigger event that is not a workflow_call + exists(Event e | + e = job.getATriggerEvent() and + not e.getName() = "workflow_call" + ) + select job, + "Actions Job or Workflow does not set permissions. Recommended minimum permissions are ($@)", + job, getMinPermissions(job) From d2f4f61264c8c0a4f983a5a6d4b49c47eb194914 Mon Sep 17 00:00:00 2001 From: Kylie Stradley <4666485+KyFaSt@users.noreply.github.com> Date: Fri, 17 Jan 2025 01:14:38 +0000 Subject: [PATCH 2/2] add missing extensions: --- actions/ql/lib/ext/config/minimum_permissions_map.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/actions/ql/lib/ext/config/minimum_permissions_map.yml b/actions/ql/lib/ext/config/minimum_permissions_map.yml index 10c8f827c237..b5ffcc5385eb 100644 --- a/actions/ql/lib/ext/config/minimum_permissions_map.yml +++ b/actions/ql/lib/ext/config/minimum_permissions_map.yml @@ -1,7 +1,7 @@ -: +extensions: - addsTo: - pack: github/actions-all - extensible: minimumPermissionsDataModel + pack: github/actions-all + extensible: minimumPermissionsDataModel data: - ["actions/cache", "{}"] - ["actions/setup-node", "contents:read"]