New commits to private projects visible in forks created while project was public
HackerOne report #1944500 by pwnie
on 2023-04-13, assigned to @greg:
Report
Summary
When a public project is forked and then privated, new commits to the repo can be viewed still.
Steps to reproduce
- Create a public project (
project 1
) ongitlab.com
withaccount 1
- On another account (
account 2
), forkproject 1
- Set
project 1
to private - Create a new file in
project 1
named "secret.txt" withsecret
inside - Go to
https://gitlab.com/account2/project2/-/compare/main...main?straight=true
(replace account2 and project2 with corresponding names) - You should see secret.txt
Impact
Upstream private repository content is not private.
P.S. This bug could potentially be chained with another bug that allows a user to create an arbitrary forked_from_project
relationship in the Project
model. Perhaps import functionality, although I was not successful in my efforts since everything seems to run through a relationship factory. Though if someone could, this would allow them to exfiltrate data from any private repository without having to fork it first, like the CTF flag you guys have in your policy for example. It would technically be possible to dump that flag if I could get forked_from_project
to be an arbitrary project.
What is the current bug behavior?
Commits from private upstream repositories are visible to forks.
What is the expected correct behavior?
Proper access controls should be implemented to prevent upstream commits from being fetched
Cause
I haven't dug that deep into why commits from upstream repositories are visible even after being privated, or why commits from upstream repositories are resolved in the first place. Though what led me to find this bug was the following code in app/models/compare.rb
:
# target == start_ref == from
def target_project
strong_memoize(:target_project) do
next source_project.default_merge_request_target unless compare_params.key?(:from_project_id)
next source_project if compare_params[:from_project_id].to_i == source_project.id
target_project = target_projects(source_project).find_by_id(compare_params[:from_project_id])
# Just ignore the field if it points at a non-existent or hidden project
next source_project unless target_project && can?(current_user, :read_code, target_project)
target_project
end
end
# source == head_ref == to
def source_project
strong_memoize(:source_project) do
# Eager load project's avatar url to prevent batch loading
# for all forked projects
project&.tap(&:avatar_url)
end
end
def compare
return [@]compare if defined?([@]compare)
[@]compare = CompareService.new(source_project, head_ref).execute(target_project, start_ref, straight: straight)
end
Notice next source_project.default_merge_request_target unless compare_params.key?(:from_project_id)
returns a default value if from_project_id
is not present. The compare controller passes from_project_id
along by default and fails to load the commits at target_project && can?(current_user, :read_code, target_project)
since we wouldn't have :read_code
permissions on a private project.
Simply removing :from_project_id
loads the default_merge_request_target
with no access controls, which is ironically the same value as the default from_project_id
. That of course gets passed to CompareService.new
and the rest is self explanatory.
Impact
Upstream private repository content is not private.
P.S. This bug could potentially be chained with another bug that allows a user to create an arbitrary forked_from_project
relationship in the Project
model. Perhaps import functionality, although I was not successful in my efforts since everything seems to run through a relationship factory. Though if someone could, this would allow them to exfiltrate data from any private repository without having to fork it first, like the CTF flag you guys have in your policy for example. It would technically be possible to dump that flag if I could get forked_from_project
to be an arbitrary project.
How To Reproduce
Please add reproducibility information to this section: