aido / app-seed-tool

A Ledger application that provides some useful seed management utilities
Apache License 2.0
42 stars 2 forks source link

Unsafe release workflow #37

Closed bboilot-ledger closed 3 months ago

bboilot-ledger commented 3 months ago

Describe the bug The release workflow is triggered by a "workflow_run" event. So anyone doing a pull request can trigger this workflow. The only restriction is that the branch starts with a "v" letter which is easy because the branch name is controlled by the contributor. So any contributor could create a release containing is changes.

Expected behavior Contributors should not be able to generate releases or control workflows with privileges (write access to repo).

Possible fix One way could be to ensure that this workflow only run from a branch and cannot be triggered by a PR. Another way could be to trigger this workflow on tag push.

aido commented 3 months ago

Hi @bboilot-ledger,

Good point! I have updated the conditions for when a release workflow is triggered. It will now only trigger for pushes to the 'develop' branch.

aido commented 3 months ago

Hi again @bboilot-ledger,

This was bugging me so I looked at it again.

The Create Release workflow is triggered on completion of the Compilation & tests workflow:

name: Create Release

on:
  workflow_run:
    workflows: ['Compilation & tests']
    types:
      - completed

The Compilation & tests workflow only triggers on pushes to the develop branch:

name: Compilation & tests

on:
  workflow_dispatch:
  push:
    branches:
      - master
      - develop

So I don't think Create Release was actually an unsafe workflow. I will leave the recent update in place nonetheless.

bboilot-ledger commented 3 months ago

Hi @aido,

From my understanding, you could add new workflow with the name Compilation & tests when opening the PR, with the wanted trigger conditions such as pull_request.
Of course, this kind of new workflow does not have any privileges (no write access to your repository). But the success of this workflow will result on the run of Create Release one which does have more privileges. The issue is also related to the fact that this workflow will checkout on the PR code which should be not trusted.

If you're interested there is an article talking about it: https://www.legitsecurity.com/blog/github-privilege-escalation-vulnerability (the most important quote to keep in mind is You should never check out user code in workflow_run context.)