Academy

Prevent a Malicious Contribution

by: Rob Tuley

Threat modelling improves security by identifying threats so you can prevent them. This post models the threat of a malicious contribution to your repository – the addition of some code with bad intent.

We’ll call the person adding the malicious code the threat actor (TA). We’ll assume they have write access to your repository. Maybe there was an access misconfiguration, or the leak of a unsuspecting user’s access token. How that might happen is the topic of another post!

This post explains how the TA would use that access, and how you can stop them.

Branches That Matter

You use branches to separate code changes from each other while you work on them. You merge branches into each other – often via a pull request – to add changes together and release them.

You can’t stop a threat actor with write access committing to your repository. They can create their own branches and commit whatever they like.

However, only some of your branches actually matter. Often it’s only 1 branch that matters – the “default” branch called main or master. The branches that matter to you depend on your branching model choices, which branches you release, and what you run in your CI pipeline. It’s these branches you need to protect from the TA.

In this post, we’ll consider the TA to be successful if they manage to commit code to the main branch.

No Branch Protection

With no branch protection, a TA with write access can simply commit malicious code directly to main. They might try to hide their actions by linking their commits to a different user. They might force push and modify previous git history to make it difficult to locate and revert their changes.

Use branch protection on all branches that matter. Even if you don’t require change approvals, you still need branch protection to prevent force push and/or branch deletion (we call this Audited Source).

Bypass Branch Protection

The threat actor’s access may be able to bypass the branch protection on main:

  • Repository admins can edit branch protection settings via the GitHub UI.
  • Repository admins and write deploy keys can bypass required approvals by default unless specifically prevented.
  • You may have configured some users or apps to bypass your protections as part of your normal workflow.

If the TA is one of those who can bypass your branch protection – they can commit malicious code directly to main.

You should:

  • Use config-as-code to define branch protections, so changing them requires approval. In Flutter-Global, this is Codebase Governor.
  • Apply your protections to everyone. Include repository admins, and avoid workflows that require users or apps to bypass them.

Hijack An Existing PR

The TA may append malicious changes to an existing pull request raised by another contributor.

  • If new commits don’t dismiss existing pull request approvals, the TA appends their changes to an already approved PR. Since the PR remains approved despite these extra changes, the TA can immediately merge the PR into main.
  • If review of the last push is possible by the person who pushed it, the TA appends changes to an existing PR. They then approve that PR themselves and merge it into main.

Prevent hijacking of existing PRs by configuring your branch protection to:

  • Dismiss stale pull request approvals when new commits are pushed.
  • Require approval of the most recent reviewable push.

Sock Puppet Approval

The TA may have access to more than one account with write access to your repository. If so, they can create a malicious pull request with one account, and approve it with the other account.

The GitHub Actions user can create and approve PRs by default. Anyone with write access to a repository can author and run a workflow with this user. Unless you change this, the TA can use the GitHub Actions user as their 2nd account. For example they can create a malicious PR via a workflow and then approve and merge it with their normal account.

You should:

  • Disable GitHub Actions creating or approving PRs unless necessary.
  • Use a CODEOWNERS file and require their approval to merge a PR. This limits the impact of PR approval from accounts you don’t normally expect it from.

Hide the Malicious Code

The TA can hide malicious code in an otherwise useful PR, and try to trick one of the repository maintainers to approve it. The TA may:

  • Create a PR with a large diff that might not be carefully reviewed – e.g. updating a set of dependencies, mocks or test code.
  • Link their commits to a different user – e.g. they could create a fake dependabot PR.
  • Obscure the malicious code addition itself – e.g. introduce a new dependency that looks OK but hides malicious code or allows adding it later.

You should:

  • Require signed commits. This can be awkward for each user to setup, but verifies commits come from those you expect.
  • Easier said than done – but review each and every PR carefully!

At Flutter

At Flutter we typically use a source control workflow we call reviewed source. This workflow includes many of the security best practices described to reduce the risk of a malicious code contribution.

  • Protect all branches that matter.
  • Use config-as-code to define your branch protection. In Flutter-Global, this is Codebase Governor.
  • Apply your protections to everyone, including repo admins.
  • Dismiss stale pull request approvals & require approval of the last push.
  • Disable GitHub Actions creating or approving PRs.
  • Use a CODEOWNERS file and require their approval.
  • Require signed commits.
  • Educate users on the importance of diligent PR review.
by: Rob Tuley
in:
tags: Branching Pull Requests Threat
category: Academy