It was introduced with the express intent to prevent someone responding to a code review request from sneaking in changes and approving them themselves or using the already supplied approval from another reviewer.
The security research that explores these topics has not been broadly published, but there are some great discussions with recommendations on how to secure your branches.
With this new policy enabled, when a reviewer applies some suggestions to the code, they can't approve and merge the code without finding another person to review their changes.
Excerpts from the article, red team, emphasis mine:
— Modify code after review
After the attacker submits a valid and good code change that is approved, the attacker abuses their existing approval to make further changes that include bad code while retaining the stale approval.
Another scenario is that the attacker could first be a good samaritan and approve the code of a fellow developer, let’s assume it’s a good code change, but it doesn’t matter. What matters is that once they have approved that pull request, they could abuse their own write access, add bad code and self-approve their own code change.
And the protections, blue team, emphasis mine:
Require a pull request before merging
- Require approvals
- Dismiss stale pull request approvals when new commits are pushed
- Require review from Code Owners
- Allow specified actors to bypass required pull requests (avoid unless you absolutely need to)
- Require approval of the most recent push (this is a new setting, as of October 2022, and is really great mitigation for some of our attack scenarios)
- Require status checks to pass before merging (it you have some form of CI with tests, linters, SAST, it would be great to enforce those)
- Require signed commits (this is great for end-to-end accountability)
- Enforce Branch Protection for administrator (i.e. “Do not allow bypassing the above settings”)
Recommended mitigations, emphasis mine:
— Modify code after review
Attacker submits good code, gets approval, then submits bad code
- The mitigation is to set your Branch Protection to “Dismiss stale pull request approvals when new commits are pushed”.
_ Attacker approves someone else’s good code, then submits bad code and self-approves changes
- The mitigation is to set your Branch Protection to “Require approval of the most recent push”.
Top comments (0)