Set Global timeout for Regexp to prevent ReDOS
What does this MR do and why?
Set Global timeout for Regexp to prevent ReDOS
Ruby version 3.2 and above provides global configuration to prevent ReDoS by setting timeout. While we should still avoid writing vulnerable regular expressions, this will significantly reduce the attack surface.
Changelog: security
MR acceptance checklist
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Screenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
Before | After |
---|---|
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
Merge request reports
Activity
assigned to @tachyons-gitlab
- A deleted user
added backend bugvulnerability typebug labels
2 Warnings 1589de4d: The commit subject must contain at least 3 words. For more information, take a look at our Commit message guidelines. b546ce72: The commit subject must contain at least 3 words. For more information, take a look at our Commit message guidelines. Reviewer roulette
Category Reviewer Maintainer backend @tyleramos
(UTC-4, 9.5 hours behind author)
@schin1
(UTC+8, 2.5 hours ahead of author)
Please refer to documentation page for guidance on how you can benefit from the Reviewer Roulette, or use the GitLab Review Workload Dashboard to find other available reviewers.
If needed, you can retry the
danger-review
job that generated this comment.Generated by
Dangerchanged milestone to %16.10
added security typemaintenance labels and removed backend bugvulnerability typebug labels
E2E Test Result Summary
allure-report-publisher
generated test report!e2e-package-and-test:
test report for 1bb82e7bexpand test summary
+---------------------------------------------------------------------+ | suites summary | +----------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +----------------+--------+--------+---------+-------+-------+--------+ | Plan | 243 | 3 | 13 | 0 | 259 | ❌ | | Govern | 266 | 6 | 19 | 0 | 291 | ❌ | | Package | 226 | 0 | 17 | 0 | 243 | ✅ | | Create | 558 | 10 | 73 | 0 | 641 | ❌ | | Systems | 8 | 0 | 0 | 0 | 8 | ✅ | | Verify | 148 | 2 | 27 | 0 | 177 | ❌ | | Data Stores | 117 | 2 | 28 | 0 | 147 | ❌ | | Fulfillment | 8 | 0 | 75 | 0 | 83 | ✅ | | Manage | 39 | 0 | 11 | 0 | 50 | ✅ | | Configure | 1 | 0 | 9 | 0 | 10 | ✅ | | Analytics | 7 | 0 | 0 | 0 | 7 | ✅ | | GitLab Metrics | 2 | 0 | 1 | 0 | 3 | ✅ | | Monitor | 36 | 0 | 13 | 0 | 49 | ✅ | | Release | 15 | 0 | 3 | 0 | 18 | ✅ | | Growth | 0 | 0 | 6 | 0 | 6 | ➖ | | Secure | 6 | 0 | 3 | 0 | 9 | ✅ | | Ai-powered | 0 | 0 | 3 | 0 | 3 | ➖ | | ModelOps | 0 | 0 | 3 | 0 | 3 | ➖ | +----------------+--------+--------+---------+-------+-------+--------+ | Total | 1680 | 23 | 304 | 0 | 2007 | ❌ | +----------------+--------+--------+---------+-------+-------+--------+
e2e-test-on-gdk:
test report for 1bb82e7bexpand test summary
+------------------------------------------------------------------+ | suites summary | +-------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +-------------+--------+--------+---------+-------+-------+--------+ | Create | 60 | 0 | 9 | 0 | 69 | ✅ | | Verify | 36 | 0 | 0 | 0 | 36 | ✅ | | Plan | 53 | 0 | 0 | 0 | 53 | ✅ | | Data Stores | 31 | 0 | 0 | 0 | 31 | ✅ | | Package | 24 | 0 | 2 | 0 | 26 | ✅ | | Govern | 66 | 0 | 0 | 0 | 66 | ✅ | | Analytics | 2 | 0 | 0 | 0 | 2 | ✅ | | Manage | 0 | 0 | 1 | 0 | 1 | ➖ | | Monitor | 7 | 0 | 0 | 0 | 7 | ✅ | | Release | 5 | 0 | 0 | 0 | 5 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+ | Total | 284 | 0 | 12 | 0 | 296 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+
e2e-review-qa:
test report for 1bb82e7bexpand test summary
+------------------------------------------------------------------+ | suites summary | +-------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +-------------+--------+--------+---------+-------+-------+--------+ | Plan | 3 | 0 | 1 | 0 | 4 | ✅ | | Govern | 3 | 0 | 0 | 0 | 3 | ✅ | | Create | 8 | 0 | 3 | 0 | 11 | ✅ | | Monitor | 4 | 0 | 0 | 0 | 4 | ✅ | | Package | 0 | 0 | 1 | 0 | 1 | ➖ | | Data Stores | 2 | 0 | 0 | 0 | 2 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+ | Total | 20 | 0 | 5 | 0 | 25 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+
Finally we're able to use this
Thanks for the MR @tachyons-gitlabIf anyone else reading this isn't familiar with our 3.2 timeline, in &9684 (closed) we mention planning to use 3.2 in production for %17.2
FYI @gitlab-com/gl-security/appsec 2 things:
- Regex DoS is about to be much less of a problem
- Until we use 3.2 in production, you might need to comment out this line when trying to reproduce regex DoS issues locally
- Resolved by Stan Hu
@dcouture
, thanks for approving this merge request.This is the first time the merge request has been approved. To ensure we don't only run predictive pipelines, and we don't break
master
, a new pipeline will be started shortly.Please wait for the pipeline to start before resolving this discussion and set auto-merge for the new pipeline. See merging a merge request for more details.
added pipeline:mr-approved label
- A deleted user
added backend label
added groupauthentication label
- Resolved by Stan Hu
removed review request for @stanhu
added devopsgovern sectionsec labels
mentioned in epic &9684 (closed)
mentioned in issue #455013 (closed)
mentioned in merge request !147971 (merged)
mentioned in issue #460517 (closed)
mentioned in merge request !154561 (merged)
changed milestone to %17.3
added 24080 commits
-
38645c22...1587998e - 24078 commits from branch
master
- 1fc9ecad - Set Global timeout for Regexp to prevent ReDOS
- b7a92364 - Update regexp.rb
-
38645c22...1587998e - 24078 commits from branch
requested review from @stanhu
- Resolved by Stan Hu
removed review request for @stanhu
requested review from @stanhu
Security policy violations have been resolved.
Edited by GitLab Security Botmentioned in issue #198688
mentioned in issue gitlab-org/quality/triage-reports#19557 (closed)
mentioned in issue gitlab-org/quality/triage-reports#19886 (closed)
added 13542 commits
Toggle commit listadded 888 commits
Toggle commit listchanged milestone to %17.5
mentioned in issue #497573 (closed)
mentioned in issue gitlab-com/gl-security/product-security/appsec/sast-custom-rules#45
added pipelinetier-3 pipeline:run-e2e-omnibus-once labels
started a merge train
mentioned in commit c39fac71
added workflowstaging-canary label
added workflowcanary label and removed workflowstaging-canary label
added workflowstaging label and removed workflowcanary label
@tachyons-gitlab Thanks for this change. Do we have any follow-up issue/task to set
REGEXP_TIMEOUT_SECONDS
to a lower number?@furkanayhan No, we don't have any yet.
I've created #499848 just now @furkanayhan @tachyons-gitlab
added workflowproduction label and removed workflowstaging label
added workflowpost-deploy-db-staging label and removed workflowproduction label
mentioned in issue #499848
mentioned in issue #499861 (closed)
mentioned in merge request !169165 (merged)
added releasedcandidate label
mentioned in merge request !174107 (merged)
mentioned in merge request !174854 (merged)
added releasedpublished label and removed releasedcandidate label
added Category:System Access label
mentioned in merge request !177104 (merged)
mentioned in merge request !177154 (merged)