Backend: Rebasing MR as Maintainer leaks parent project variables

HackerOne report #1512621 by epirat on 2022-03-15, assigned to @vdesousa:

Report | How To Reproduce

Report

Note: This is essentially the same as GitLab Bug #354115 but I did not get any reply so far and I am unable to set the correct tags on the issue so I feel like it maybe fell through the cracks, hence submitting here as is advised for security issues in the GitLab Docs.

Summary

When a Maintainer rebases an MR (and using detached pipelines), the rebased MR will trigger a detached pipeline that runs
in the context of the parent project, not isolated to the MR and is able to access variables from the parent project.

Steps to reproduce
  1. Create a variable in the project, e.g. MY_SECRET with the value 42 and make sure that the MR merge stategy is set to FF.
  2. Have a (non-maintainer) create a Fork, add a variable too, like MY_SECRET with the value 1.
  3. Create a MR, for example one that adds the following CI file:
stages:  
  - build

build-job:  
  stage: build  
  script:  
    - echo "This is an example job..."  
    - '[ "$MY_SECRET" -eq 42 ] && echo "Oh no!" || echo "Ok"'  
    - echo "The end."

workflow:  
  rules:  
    - if: $CI_PIPELINE_SOURCE == 'merge_request_event'

This will trigger CI as expected and run the job in the context of the fork,
so it should print "OK".

  1. Now have a maintainer make a change to main and then rebase the MR by clicking the rebase button

While it still shows the pipeline as detached, it actually runs in the context of the parent project
and has access to the variable, printing "Oh no!" in this case.

Impact

If a Maintainer is not careful to verify a change has no possibly malicious accesses to secrets it should not have, and just rebases (as there is no warning whatsoever), it can lead to information disclosure that possibly includes credentials like passwords as these are frequently stored in CI variables.

Examples

You can find an example of this behavior in our GitLab instance here:

https://code.videolan.org/test-projects/test1/-/merge_requests/2/pipelines

What is the current bug behavior?

A maintainer rebasing a MR should not cause the pipeline to suddenly get access to the
variables of the parent project as that is not clear that it happens and not detailed on
the CI/CD variables security section at

https://docs.gitlab.com/ee/ci/variables/#cicd-variable-security

either. The only two cases covered there are:

Review all merge requests that introduce changes to the .gitlab-ci.yml file before you:

  • Run a pipeline in the parent project for a merge request submitted from a forked project.
  • Merge the changes.

The former case is documented to require the following actions:

To run a pipeline in the parent project for a merge request from a fork project:

  • In the merge request, go to the Pipelines tab.
  • Select Run pipeline. You must accept the warning, or the pipeline does not run.

However this is not what is done here to trigger this issue. There is no warning whatsoever
for what is about to happen when the Maintainer rebases the MR, that this will expose possibly
secrets from the parent projects CI/CD variables to this MR.

What is the expected correct behavior?

The warning documented is displayed and must be accepted before rebasing the MR.

OR

Rebasing the MR, no matter the role of the person who clicks the rebase button, should have
consistent behavior and not expose parent projects CI/CD variables or similar resources.

Of course it can be useful to have this behavior but then it need explicit UI and an
explicit action by the Maintainer with an appropriate warning. And for such pipelines
the UI should display properly that those run in the parent context, currently it
has the "detached" label on these with the following description:

Merge request pipelines are configured. A detached pipeline runs in the context of the merge request, and not against the merged result. Learn more in the documentation for merge request pipelines.

This definitely suggests none of what we experience here should happen for such a pipeline.

Relevant logs and/or screenshots

See the linked repository/MR for relevant logs.

https://code.videolan.org/test-projects/test1/-/merge_requests/2/pipelines

Output of checks
Results of GitLab environment info

I do not have access to the GitLab instance to gather these, but it should reproduce the
same on GitLab.com

Our tests for this issue were conducted with our self-hosted GitLab 14.8.2.

Impact

If a Maintainer is not careful to verify a change does no possibly malicious access to secrets and just rebases (as there is no warning whatsoever), it can lead to information disclosure that possibly includes credentials like passwords as these are frequently stored in CI variables.

How To Reproduce

Please add reproducibility information to this section:

Proposal

Summary

The fix we will pursue for this is front end to show warnings everywhere.

  • Run pipeline from the UI
  • /rebase quick action
  • Run pipeline for merge request via API
  • Rebase via API
  • Changes pushed by the user to the source branch

Work to do

  • Add a use case to lib/gitlab/quick_actions/merge_request_actions.rb for the warning for the quick action
  • Add security modal when rebasing via the UI
  • Validate run pipeline security modal via the UI
  • Validate API error message is descriptive for those use cases.
Edited by Payton Burdette