Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: composite action support #331

Merged
merged 32 commits into from
Dec 25, 2024
Merged

feat: composite action support #331

merged 32 commits into from
Dec 25, 2024

Conversation

woodruffw
Copy link
Owner

@woodruffw woodruffw commented Dec 19, 2024

Very WIP, just stitching things together.

Closes #173.

Some key limitations:

  • The ignore config doesn't currently support ignoring composite actions

Very WIP, just stitching things together.

Signed-off-by: William Woodruff <william@yossarian.net>
@woodruffw
Copy link
Owner Author

I've started working on this, but I'm now realizing that the trait layout here isn't going to be conducive to a separate ActionAudit, since Rust doesn't have a clean way to cast between traits.

Instead of keeping WorkflowAudit and ActionAudit separate, I think I'm going to rename Audit -> AuditCore and then make WorkflowAudit into a single Audit trait that can handle both workflows and actions. That'll avoid the trait casting pain.

Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
@woodruffw woodruffw self-assigned this Dec 21, 2024
@woodruffw woodruffw added the enhancement New feature or request label Dec 21, 2024
@woodruffw
Copy link
Owner Author

There's still a decent amount of cleanup that needs to be done here, but zizmor can now audit actions! Here's what it produces for the vulnerable Ultralytics action (https://github.com/ultralytics/actions/blob/63d2cd98a098f45d5532e3b1ee7715bae66280c2/action.yml):

error[template-injection]: code injection via template expansion
  --> /Users/william/tmp/action.yml:70:7
   |
70 |       - name: Install Dependencies
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^ this step
71 |         # Note tomli required for codespell with pyproject.toml
...
74 |         #   pip install -q git+https://github.com/ultralytics/actions@main codespell tomli
75 | /       run: |
76 | |         packages="ultralytics-actions"
...  |
87 | |
88 | |         ultralytics-actions-info
   | |________________________________^ inputs.spelling may expand into attacker-controllable code
   |
   = note: audit confidence → Low

error[template-injection]: code injection via template expansion
   --> /Users/william/tmp/action.yml:211:7
    |
211 |       - name: Commit and Push Changes
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this step
212 |         if: (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && github.event.action != 'closed'
213 | /       run: |
214 | |         git config --global user.name "${{ inputs.github_username }}"
...   |
223 | |           echo "No changes to commit"
224 | |         fi
    | |__________^ inputs.github_username may expand into attacker-controllable code
    |
    = note: audit confidence → Low

error[template-injection]: code injection via template expansion
   --> /Users/william/tmp/action.yml:211:7
    |
211 |       - name: Commit and Push Changes
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this step
212 |         if: (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && github.event.action != 'closed'
213 | /       run: |
214 | |         git config --global user.name "${{ inputs.github_username }}"
...   |
223 | |           echo "No changes to commit"
224 | |         fi
    | |__________^ inputs.github_email may expand into attacker-controllable code
    |
    = note: audit confidence → Low

error[template-injection]: code injection via template expansion
   --> /Users/william/tmp/action.yml:211:7
    |
211 |       - name: Commit and Push Changes
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this step
212 |         if: (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && github.event.action != 'closed'
213 | /       run: |
214 | |         git config --global user.name "${{ inputs.github_username }}"
...   |
223 | |           echo "No changes to commit"
224 | |         fi
    | |__________^ github.head_ref may expand into attacker-controllable code
    |
    = note: audit confidence → High

error[template-injection]: code injection via template expansion
   --> /Users/william/tmp/action.yml:211:7
    |
211 |       - name: Commit and Push Changes
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this step
212 |         if: (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && github.event.action != 'closed'
213 | /       run: |
214 | |         git config --global user.name "${{ inputs.github_username }}"
...   |
223 | |           echo "No changes to commit"
224 | |         fi
    | |__________^ github.ref may expand into attacker-controllable code
    |
    = note: audit confidence → High

5 findings: 0 unknown, 0 informational, 0 low, 0 medium, 5 high

Critically, it catches all three of the template injections (the last 3 findings) that made the action exploitable.

Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
@woodruffw woodruffw marked this pull request as ready for review December 23, 2024 19:25
@woodruffw woodruffw merged commit c28d44c into main Dec 25, 2024
5 checks passed
@woodruffw woodruffw deleted the ww/composite-actions branch December 25, 2024 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: support auditing (composite) actions
1 participant