1--- 2type: reference, dev 3stage: none 4group: Development 5info: "See the Technical Writers assigned to Development Guidelines: https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments-to-development-guidelines" 6--- 7 8# Secure Coding Guidelines 9 10This document contains descriptions and guidelines for addressing security 11vulnerabilities commonly identified in the GitLab codebase. They are intended 12to help developers identify potential security vulnerabilities early, with the 13goal of reducing the number of vulnerabilities released over time. 14 15**Contributing** 16 17If you would like to contribute to one of the existing documents, or add 18guidelines for a new vulnerability type, please open an MR! Please try to 19include links to examples of the vulnerability found, and link to any resources 20used in defined mitigations. If you have questions or when ready for a review, 21please ping `gitlab-com/gl-security/appsec`. 22 23## Permissions 24 25### Description 26 27Application permissions are used to determine who can access what and what actions they can perform. 28For more information about the permission model at GitLab, please see [the GitLab permissions guide](permissions.md) or the [EE docs on permissions](../../ee/user/permissions.md). 29 30### Impact 31 32Improper permission handling can have significant impacts on the security of an application. 33Some situations may reveal [sensitive data](https://gitlab.com/gitlab-com/gl-infra/production/-/issues/477) or allow a malicious actor to perform [harmful actions](https://gitlab.com/gitlab-org/gitlab/-/issues/8180). 34The overall impact depends heavily on what resources can be accessed or modified improperly. 35 36A common vulnerability when permission checks are missing is called [IDOR](https://owasp.org/www-project-web-security-testing-guide/latest/4-Web_Application_Security_Testing/05-Authorization_Testing/04-Testing_for_Insecure_Direct_Object_References) for Insecure Direct Object References. 37 38### When to Consider 39 40Each time you implement a new feature/endpoint, whether it is at UI, API or GraphQL level. 41 42### Mitigations 43 44**Start by writing tests** around permissions: unit and feature specs should both include tests based around permissions 45 46- Fine-grained, nitty-gritty specs for permissions are good: it is ok to be verbose here 47 - Make assertions based on the actors and objects involved: can a user or group or XYZ perform this action on this object? 48 - Consider defining them upfront with stakeholders, particularly for the edge cases 49- Do not forget **abuse cases**: write specs that **make sure certain things can't happen** 50 - A lot of specs are making sure things do happen and coverage percentage doesn't take into account permissions as same piece of code is used. 51 - Make assertions that certain actors cannot perform actions 52- Naming convention to ease auditability: to be defined, for example, a subfolder containing those specific permission tests or a `#permissions` block 53 54Be careful to **also test [visibility levels](https://gitlab.com/gitlab-org/gitlab-foss/-/blob/master/doc/development/permissions.md#feature-specific-permissions)** and not only project access rights. 55 56Some example of well implemented access controls and tests: 57 581. [example1](https://dev.gitlab.org/gitlab/gitlab-ee/-/merge_requests/710/diffs?diff_id=13750#af40ef0eaae3c1e018809e1d88086e32bccaca40_43_43) 591. [example2](https://dev.gitlab.org/gitlab/gitlabhq/-/merge_requests/2511/diffs#ed3aaab1510f43b032ce345909a887e5b167e196_142_155) 601. [example3](https://dev.gitlab.org/gitlab/gitlabhq/-/merge_requests/3170/diffs?diff_id=17494) 61 62**NB:** any input from development team is welcome, for example, about Rubocop rules. 63 64## Regular Expressions guidelines 65 66### Anchors / Multi line 67 68Unlike other programming languages (for example, Perl or Python) Regular Expressions are matching multi-line by default in Ruby. Consider the following example in Python: 69 70```python 71import re 72text = "foo\nbar" 73matches = re.findall("^bar$",text) 74print(matches) 75``` 76 77The Python example will output an empty array (`[]`) as the matcher considers the whole string `foo\nbar` including the newline (`\n`). In contrast Ruby's Regular Expression engine acts differently: 78 79```ruby 80text = "foo\nbar" 81p text.match /^bar$/ 82``` 83 84The output of this example is `#<MatchData "bar">`, as Ruby treats the input `text` line by line. In order to match the whole __string__ the Regex anchors `\A` and `\z` should be used. 85 86#### Impact 87 88This Ruby Regex specialty can have security impact, as often regular expressions are used for validations or to impose restrictions on user-input. 89 90#### Examples 91 92GitLab-specific examples can be found in the following [path traversal](https://gitlab.com/gitlab-org/gitlab/-/issues/36029#note_251262187) 93and [open redirect](https://gitlab.com/gitlab-org/gitlab/-/issues/33569) issues. 94 95Another example would be this fictional Ruby on Rails controller: 96 97```ruby 98class PingController < ApplicationController 99 def ping 100 if params[:ip] =~ /^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/ 101 render :text => `ping -c 4 #{params[:ip]}` 102 else 103 render :text => "Invalid IP" 104 end 105 end 106end 107``` 108 109Here `params[:ip]` should not contain anything else but numbers and dots. However this restriction can be easily bypassed as the Regex anchors `^` and `$` are being used. Ultimately this leads to a shell command injection in `ping -c 4 #{params[:ip]}` by using newlines in `params[:ip]`. 110 111#### Mitigation 112 113In most cases the anchors `\A` for beginning of text and `\z` for end of text should be used instead of `^` and `$`. 114 115## Denial of Service (ReDoS) / Catastrophic Backtracking 116 117When a regular expression (regex) is used to search for a string and can't find a match, 118it may then backtrack to try other possibilities. 119 120For example when the regex `.*!$` matches the string `hello!`, the `.*` first matches 121the entire string but then the `!` from the regex is unable to match because the 122character has already been used. In that case, the Ruby regex engine _backtracks_ 123one character to allow the `!` to match. 124 125[ReDoS](https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS) 126is an attack in which the attacker knows or controls the regular expression used. 127The attacker may be able to enter user input that triggers this backtracking behavior in a 128way that increases execution time by several orders of magnitude. 129 130### Impact 131 132The resource, for example Puma, or Sidekiq, can be made to hang as it takes 133a long time to evaluate the bad regex match. The evaluation time may require manual 134termination of the resource. 135 136### Examples 137 138Here are some GitLab-specific examples. 139 140User inputs used to create regular expressions: 141 142- [User-controlled filename](https://gitlab.com/gitlab-org/gitlab/-/issues/257497) 143- [User-controlled domain name](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/25314) 144- [User-controlled email address](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/25122#note_289087459) 145 146Hardcoded regular expressions with backtracking issues: 147 148- [Repository name validation](https://gitlab.com/gitlab-org/gitlab/-/issues/220019) 149- [Link validation](https://gitlab.com/gitlab-org/gitlab/-/issues/218753), and [a bypass](https://gitlab.com/gitlab-org/gitlab/-/issues/273771) 150- [Entity name validation](https://gitlab.com/gitlab-org/gitlab/-/issues/289934) 151- [Validating color codes](https://gitlab.com/gitlab-org/gitlab/commit/717824144f8181bef524592eab882dd7525a60ef) 152 153Consider the following example application, which defines a check using a regular expression. A user entering `user@aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!.com` as the email on a form will hang the web server. 154 155```ruby 156class Email < ApplicationRecord 157 DOMAIN_MATCH = Regexp.new('([a-zA-Z0-9]+)+\.com') 158 159 validates :domain_matches 160 161 private 162 163 def domain_matches 164 errors.add(:email, 'does not match') if email =~ DOMAIN_MATCH 165 end 166end 167``` 168 169### Mitigation 170 171#### Ruby 172 173GitLab has [`Gitlab::UntrustedRegexp`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/untrusted_regexp.rb) 174 which internally uses the [`re2`](https://github.com/google/re2/wiki/Syntax) library. 175`re2` does not support backtracking so we get constant execution time, and a smaller subset of available regex features. 176 177All user-provided regular expressions should use `Gitlab::UntrustedRegexp`. 178 179For other regular expressions, here are a few guidelines: 180 181- If there's a clean non-regex solution, such as `String#start_with?`, consider using it 182- Ruby supports some advanced regex features like [atomic groups](https://www.regular-expressions.info/atomic.html) 183and [possessive quantifiers](https://www.regular-expressions.info/possessive.html) that eliminate backtracking 184- Avoid nested quantifiers if possible (for example `(a+)+`) 185- Try to be as precise as possible in your regex and avoid the `.` if there's an alternative 186 - For example, Use `_[^_]+_` instead of `_.*_` to match `_text here_` 187- If in doubt, don't hesitate to ping `@gitlab-com/gl-security/appsec` 188 189#### Go 190 191Go's [`regexp`](https://pkg.go.dev/regexp) package uses `re2` and isn't vulnerable to backtracking issues. 192 193## Further Links 194 195- [Rubular](https://rubular.com/) is a nice online tool to fiddle with Ruby Regexps. 196- [Runaway Regular Expressions](https://www.regular-expressions.info/catastrophic.html) 197- [The impact of regular expression denial of service (ReDoS) in practice: an empirical study at the ecosystem scale](https://people.cs.vt.edu/~davisjam/downloads/publications/DavisCoghlanServantLee-EcosystemREDOS-ESECFSE18.pdf). This research paper discusses approaches to automatically detect ReDoS vulnerabilities. 198- [Freezing the web: A study of ReDoS vulnerabilities in JavaScript-based web servers](https://www.usenix.org/system/files/conference/usenixsecurity18/sec18-staicu.pdf). Another research paper about detecting ReDoS vulnerabilities. 199 200## Server Side Request Forgery (SSRF) 201 202### Description 203 204A [Server-side Request Forgery (SSRF)](https://www.hackerone.com/blog-How-To-Server-Side-Request-Forgery-SSRF) is an attack in which an attacker 205is able coerce a application into making an outbound request to an unintended 206resource. This resource is usually internal. In GitLab, the connection most 207commonly uses HTTP, but an SSRF can be performed with any protocol, such as 208Redis or SSH. 209 210With an SSRF attack, the UI may or may not show the response. The latter is 211called a Blind SSRF. While the impact is reduced, it can still be useful for 212attackers, especially for mapping internal network services as part of recon. 213 214### Impact 215 216The impact of an SSRF can vary, depending on what the application server 217can communicate with, how much the attacker can control of the payload, and 218if the response is returned back to the attacker. Examples of impact that 219have been reported to GitLab include: 220 221- Network mapping of internal services 222 - This can help an attacker gather information about internal services 223 that could be used in further attacks. [More details](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/51327). 224- Reading internal services, including cloud service metadata. 225 - The latter can be a serious problem, because an attacker can obtain keys that allow control of the victim's cloud infrastructure. (This is also a good reason 226 to give only necessary privileges to the token.). [More details](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/51490). 227- When combined with CRLF vulnerability, remote code execution. [More details](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/41293). 228 229### When to Consider 230 231- When the application makes any outbound connection 232 233### Mitigations 234 235In order to mitigate SSRF vulnerabilities, it is necessary to validate the destination of the outgoing request, especially if it includes user-supplied information. 236 237The preferred SSRF mitigations within GitLab are: 238 2391. Only connect to known, trusted domains/IP addresses. 2401. Use the [GitLab::HTTP](#gitlab-http-library) library 2411. Implement [feature-specific mitigations](#feature-specific-mitigations) 242 243#### GitLab HTTP Library 244 245The [GitLab::HTTP](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/http.rb) wrapper library has grown to include mitigations for all of the GitLab-known SSRF vectors. It is also configured to respect the 246`Outbound requests` options that allow instance administrators to block all internal connections, or limit the networks to which connections can be made. 247 248In some cases, it has been possible to configure GitLab::HTTP as the HTTP 249connection library for 3rd-party gems. This is preferable over re-implementing 250the mitigations for a new feature. 251 252- [More details](https://dev.gitlab.org/gitlab/gitlabhq/-/merge_requests/2530/diffs) 253 254#### Feature-specific mitigations 255 256For situations in which an allowlist or GitLab:HTTP cannot be used, it will be necessary to implement mitigations directly in the feature. It is best to validate the destination IP addresses themselves, not just domain names, as DNS can be controlled by the attacker. Below are a list of mitigations that should be implemented. 257 258There are many tricks to bypass common SSRF validations. If feature-specific mitigations are necessary, they should be reviewed by the AppSec team, or a developer who has worked on SSRF mitigations previously. 259 260- Block connections to all localhost addresses 261 - `127.0.0.1/8` (IPv4 - note the subnet mask) 262 - `::1` (IPv6) 263- Block connections to networks with private addressing (RFC 1918) 264 - `10.0.0.0/8` 265 - `172.16.0.0/12` 266 - `192.168.0.0/24` 267- Block connections to link-local addresses (RFC 3927) 268 - `169.254.0.0/16` 269 - In particular, for GCP: `metadata.google.internal` -> `169.254.169.254` 270- For HTTP connections: Disable redirects or validate the redirect destination 271- To mitigate DNS rebinding attacks, validate and use the first IP address received 272 273See [`url_blocker_spec.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/spec/lib/gitlab/url_blocker_spec.rb) for examples of SSRF payloads 274 275## XSS guidelines 276 277### Description 278 279Cross site scripting (XSS) is an issue where malicious JavaScript code gets injected into a trusted web application and executed in a client's browser. The input is intended to be data, but instead gets treated as code by the browser. 280 281XSS issues are commonly classified in three categories, by their delivery method: 282 283- [Persistent XSS](https://owasp.org/www-community/Types_of_Cross-Site_Scripting#stored-xss-aka-persistent-or-type-i) 284- [Reflected XSS](https://owasp.org/www-community/Types_of_Cross-Site_Scripting#reflected-xss-aka-non-persistent-or-type-ii) 285- [DOM XSS](https://owasp.org/www-community/Types_of_Cross-Site_Scripting#dom-based-xss-aka-type-0) 286 287### Impact 288 289The injected client-side code is executed on the victim's browser in the context of their current session. This means the attacker could perform any same action the victim would normally be able to do through a browser. The attacker would also have the ability to: 290 291- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [log victim keystrokes](https://youtu.be/2VFavqfDS6w?t=1367) 292- launch a network scan from the victim's browser 293- potentially <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [obtain the victim's session tokens](https://youtu.be/2VFavqfDS6w?t=739) 294- perform actions that lead to data loss/theft or account takeover 295 296Much of the impact is contingent upon the function of the application and the capabilities of the victim's session. For further impact possibilities, please check out [the beef project](https://beefproject.com/). 297 298For a demonstration of the impact on GitLab with a realistic attack scenario, see [this video on the GitLab Unfiltered channel](https://www.youtube.com/watch?v=t4PzHNycoKo) (internal, it requires being logged in with the GitLab Unfiltered account). 299 300### When to consider? 301 302When user submitted data is included in responses to end users, which is just about anywhere. 303 304### Mitigation 305 306In most situations, a two-step solution can be used: input validation and output encoding in the appropriate context. 307 308#### Input validation 309 310- [Input Validation](https://youtu.be/2VFavqfDS6w?t=7489) 311 312##### Setting expectations 313 314For any and all input fields, ensure to define expectations on the type/format of input, the contents, <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [size limits](https://youtu.be/2VFavqfDS6w?t=7582), the context in which it will be output. It's important to work with both security and product teams to determine what is considered acceptable input. 315 316##### Validate input 317 318- Treat all user input as untrusted. 319- Based on the expectations you [defined above](#setting-expectations): 320 - Validate the <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [input size limits](https://youtu.be/2VFavqfDS6w?t=7582). 321 - Validate the input using an <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [allowlist approach](https://youtu.be/2VFavqfDS6w?t=7816) to only allow characters through which you are expecting to receive for the field. 322 - Input which fails validation should be **rejected**, and not sanitized. 323- When adding redirects or links to a user-controlled URL, ensure that the scheme is HTTP or HTTPS. Allowing other schemes like `javascript://` can lead to XSS and other security issues. 324 325Note that denylists should be avoided, as it is near impossible to block all [variations of XSS](https://owasp.org/www-community/xss-filter-evasion-cheatsheet). 326 327#### Output encoding 328 329Once you've [determined when and where](#setting-expectations) the user submitted data will be output, it's important to encode it based on the appropriate context. For example: 330 331- Content placed inside HTML elements need to be [HTML entity encoded](https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#rule-1---html-escape-before-inserting-untrusted-data-into-html-element-content). 332- Content placed into a JSON response needs to be [JSON encoded](https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#rule-31---html-escape-json-values-in-an-html-context-and-read-the-data-with-jsonparse). 333- Content placed inside <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [HTML URL GET parameters](https://youtu.be/2VFavqfDS6w?t=3494) need to be [URL-encoded](https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#rule-5---url-escape-before-inserting-untrusted-data-into-html-url-parameter-values) 334- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [Additional contexts may require context-specific encoding](https://youtu.be/2VFavqfDS6w?t=2341). 335 336### Additional information 337 338#### XSS mitigation and prevention in Rails 339 340By default, Rails automatically escapes strings when they are inserted into HTML templates. Avoid the 341methods used to keep Rails from escaping strings, especially those related to user-controlled values. 342Specifically, the following options are dangerous because they mark strings as trusted and safe: 343 344| Method | Avoid these options | 345|----------------------|-------------------------------| 346| HAML templates | `html_safe`, `raw`, `!=` | 347| Embedded Ruby (ERB) | `html_safe`, `raw`, `<%== %>` | 348 349In case you want to sanitize user-controlled values against XSS vulnerabilities, you can use 350[`ActionView::Helpers::SanitizeHelper`](https://api.rubyonrails.org/classes/ActionView/Helpers/SanitizeHelper.html). 351Calling `link_to` and `redirect_to` with user-controlled parameters can also lead to cross-site scripting. 352 353Do also sanitize and validate URL schemes. 354 355References: 356 357- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [XSS Defense in Rails](https://youtu.be/2VFavqfDS6w?t=2442) 358- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [XSS Defense with HAML](https://youtu.be/2VFavqfDS6w?t=2796) 359- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [Validating Untrusted URLs in Ruby](https://youtu.be/2VFavqfDS6w?t=3936) 360- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [RoR Model Validators](https://youtu.be/2VFavqfDS6w?t=7636) 361 362#### XSS mitigation and prevention in JavaScript and Vue 363 364- When updating the content of an HTML element using JavaScript, mark user-controlled values as `textContent` or `nodeValue` instead of `innerHTML`. 365- Avoid using `v-html` with user-controlled data, use [`v-safe-html`](https://gitlab-org.gitlab.io/gitlab-ui/?path=/story/directives-safe-html-directive--default) instead. 366- Render unsafe or unsanitized content using [`dompurify`](fe_guide/security.md#sanitize-html-output). 367- Consider using [`gl-sprintf`](../../ee/development/i18n/externalization.md#interpolation) to interpolate translated strings securely. 368- Avoid `__()` with translations that contain user-controlled values. 369- When working with `postMessage`, ensure the `origin` of the message is allowlisted. 370- Consider using the [Safe Link Directive](https://gitlab-org.gitlab.io/gitlab-ui/?path=/story/directives-safe-link-directive--default) to generate secure hyperlinks by default. 371 372#### GitLab specific libraries for mitigating XSS 373 374##### Vue 375 376- [isSafeURL](https://gitlab.com/gitlab-org/gitlab/-/blob/v12.7.5-ee/app/assets/javascripts/lib/utils/url_utility.js#L190-207) 377- [GlSprintf](https://gitlab-org.gitlab.io/gitlab-ui/?path=/story/utilities-sprintf--default) 378 379#### Content Security Policy 380 381- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [Content Security Policy](https://www.youtube.com/watch?v=2VFavqfDS6w&t=12991s) 382- [Use nonce-based Content Security Policy for inline JavaScript](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/65330) 383 384#### Free form input field 385 386### Select examples of past XSS issues affecting GitLab 387 388- [Stored XSS in user status](https://gitlab.com/gitlab-org/gitlab-foss/issues/55320) 389- [XSS vulnerability on custom project templates form](https://gitlab.com/gitlab-org/gitlab/-/issues/197302) 390- [Stored XSS in branch names](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/55320) 391- [Stored XSS in merge request pages](https://gitlab.com/gitlab-org/gitlab/-/issues/35096) 392 393### Internal Developer Training 394 395- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [Introduction to XSS](https://www.youtube.com/watch?v=PXR8PTojHmc&t=7785s) 396- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [Reflected XSS](https://youtu.be/2VFavqfDS6w?t=603s) 397- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [Persistent XSS](https://youtu.be/2VFavqfDS6w?t=643) 398- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [DOM XSS](https://youtu.be/2VFavqfDS6w?t=5871) 399- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [XSS in depth](https://www.youtube.com/watch?v=2VFavqfDS6w&t=111s) 400- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [XSS Defense](https://youtu.be/2VFavqfDS6w?t=1685) 401- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [XSS Defense in Rails](https://youtu.be/2VFavqfDS6w?t=2442) 402- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [XSS Defense with HAML](https://youtu.be/2VFavqfDS6w?t=2796) 403- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [JavaScript URLs](https://youtu.be/2VFavqfDS6w?t=3274) 404- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [URL encoding context](https://youtu.be/2VFavqfDS6w?t=3494) 405- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [Validating Untrusted URLs in Ruby](https://youtu.be/2VFavqfDS6w?t=3936) 406- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [HTML Sanitization](https://youtu.be/2VFavqfDS6w?t=5075) 407- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [DOMPurify](https://youtu.be/2VFavqfDS6w?t=5381) 408- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [Safe Client-side JSON Handling](https://youtu.be/2VFavqfDS6w?t=6334) 409- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [iframe sandboxing](https://youtu.be/2VFavqfDS6w?t=7043) 410- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [Input Validation](https://youtu.be/2VFavqfDS6w?t=7489) 411- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [Validate size limits](https://youtu.be/2VFavqfDS6w?t=7582) 412- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [RoR model validators](https://youtu.be/2VFavqfDS6w?t=7636) 413- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [Allowlist input validation](https://youtu.be/2VFavqfDS6w?t=7816) 414- <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [Content Security Policy](https://www.youtube.com/watch?v=2VFavqfDS6w&t=12991s) 415 416## Path Traversal guidelines 417 418### Description 419 420Path Traversal vulnerabilities grant attackers access to arbitrary directories and files on the server that is executing an application, including data, code or credentials. 421 422### Impact 423 424Path Traversal attacks can lead to multiple critical and high severity issues, like arbitrary file read, remote code execution or information disclosure. 425 426### When to consider 427 428When working with user-controlled filenames/paths and file system APIs. 429 430### Mitigation and prevention 431 432In order to prevent Path Traversal vulnerabilities, user-controlled filenames or paths should be validated before being processed. 433 434- Comparing user input against an allowlist of allowed values or verifying that it only contains allowed characters. 435- After validating the user supplied input, it should be appended to the base directory and the path should be canonicalized using the file system API. 436 437#### GitLab specific validations 438 439The methods `Gitlab::Utils.check_path_traversal!()` and `Gitlab::Utils.check_allowed_absolute_path!()` 440can be used to validate user-supplied paths and prevent vulnerabilities. 441`check_path_traversal!()` will detect their Path Traversal payloads and accepts URL-encoded paths. 442`check_allowed_absolute_path!()` will check if a path is absolute and whether it is inside the allowed path list. By default, absolute 443paths are not allowed, so you need to pass a list of allowed absolute paths to the `path_allowlist` 444parameter when using `check_allowed_absolute_path!()`. 445 446To use a combination of both checks, follow the example below: 447 448```ruby 449path = Gitlab::Utils.check_path_traversal!(path) 450Gitlab::Utils.check_allowed_absolute_path!(path, path_allowlist) 451``` 452 453In the REST API, we have the [`FilePath`](https://gitlab.com/gitlab-org/security/gitlab/-/blob/master/lib/api/validations/validators/file_path.rb) 454validator that can be used to perform the checking on any file path argument the endpoints have. 455It can be used as follows: 456 457```ruby 458requires :file_path, type: String, file_path: { allowlist: ['/foo/bar/', '/home/foo/', '/app/home'] } 459``` 460 461The Path Traversal check can also be used to forbid any absolute path: 462 463```ruby 464requires :file_path, type: String, file_path: true 465``` 466 467Absolute paths are not allowed by default. If allowing an absolute path is required, you 468need to provide an array of paths to the parameter `allowlist`. 469 470## OS command injection guidelines 471 472Command injection is an issue in which an attacker is able to execute arbitrary commands on the host 473operating system through a vulnerable application. Such attacks don't always provide feedback to a 474user, but the attacker can use simple commands like `curl` to obtain an answer. 475 476### Impact 477 478The impact of command injection greatly depends on the user context running the commands, as well as 479how data is validated and sanitized. It can vary from low impact because the user running the 480injected commands has limited rights, to critical impact if running as the root user. 481 482Potential impacts include: 483 484- Execution of arbitrary commands on the host machine. 485- Unauthorized access to sensitive data, including passwords and tokens in secrets or configuration 486 files. 487- Exposure of sensitive system files on the host machine, such as `/etc/passwd/` or `/etc/shadow`. 488- Compromise of related systems and services gained through access to the host machine. 489 490You should be aware of and take steps to prevent command injection when working with user-controlled 491data that are used to run OS commands. 492 493### Mitigation and prevention 494 495To prevent OS command injections, user-supplied data shouldn't be used within OS commands. In cases 496where you can't avoid this: 497 498- Validate user-supplied data against an allowlist. 499- Ensure that user-supplied data only contains alphanumeric characters (and no syntax or whitespace 500 characters, for example). 501- Always use `--` to separate options from arguments. 502 503#### Ruby 504 505Consider using `system("command", "arg0", "arg1", ...)` whenever you can. This prevents an attacker 506from concatenating commands. 507 508For more examples on how to use shell commands securely, consult 509[Guidelines for shell commands in the GitLab codebase](shell_commands.md). 510It contains various examples on how to securely call OS commands. 511 512#### Go 513 514Go has built-in protections that usually prevent an attacker from successfully injecting OS commands. 515 516Consider the following example: 517 518```golang 519package main 520 521import ( 522 "fmt" 523 "os/exec" 524) 525 526func main() { 527 cmd := exec.Command("echo", "1; cat /etc/passwd") 528 out, _ := cmd.Output() 529 fmt.Printf("%s", out) 530} 531``` 532 533This echoes `"1; cat /etc/passwd"`. 534 535**Do not** use `sh`, as it bypasses internal protections: 536 537```golang 538out, _ = exec.Command("sh", "-c", "echo 1 | cat /etc/passwd").Output() 539``` 540 541This outputs `1` followed by the content of `/etc/passwd`. 542 543## General recommendations 544 545### TLS minimum recommended version 546 547As we have [moved away from supporting TLS 1.0 and 1.1](https://about.gitlab.com/blog/2018/10/15/gitlab-to-deprecate-older-tls/), you must use TLS 1.2 and above. 548 549#### Ciphers 550 551We recommend using the ciphers that Mozilla is providing in their [recommended SSL configuration generator](https://ssl-config.mozilla.org/#server=go&version=1.17&config=intermediate&guideline=5.6) for TLS 1.2: 552 553- `ECDHE-ECDSA-AES128-GCM-SHA256` 554- `ECDHE-RSA-AES128-GCM-SHA256` 555- `ECDHE-ECDSA-AES256-GCM-SHA384` 556- `ECDHE-RSA-AES256-GCM-SHA384` 557- `ECDHE-ECDSA-CHACHA20-POLY1305` 558- `ECDHE-RSA-CHACHA20-POLY1305` 559 560And the following cipher suites (according to the [RFC 8446](https://datatracker.ietf.org/doc/html/rfc8446#appendix-B.4)) for TLS 1.3: 561 562- `TLS_AES_128_GCM_SHA256` 563- `TLS_AES_256_GCM_SHA384` 564- `TLS_CHACHA20_POLY1305_SHA256` 565 566*Note*: **Golang** does [not support](https://github.com/golang/go/blob/go1.17/src/crypto/tls/cipher_suites.go#L676) all cipher suites with TLS 1.3. 567 568##### Implementation examples 569 570##### TLS 1.3 571 572For TLS 1.3, **Golang** only supports [3 cipher suites](https://github.com/golang/go/blob/go1.17/src/crypto/tls/cipher_suites.go#L676), as such we only need to set the TLS version: 573 574```golang 575cfg := &tls.Config{ 576 MinVersion: tls.VersionTLS13, 577} 578``` 579 580For **Ruby**, you can use [HTTParty](https://github.com/jnunemaker/httparty) and specify TLS 1.3 version as well as ciphers: 581 582Whenever possible this example should be **avoided** for security purposes: 583 584```ruby 585response = HTTParty.get('https://gitlab.com', ssl_version: :TLSv1_3, ciphers: ['TLS_AES_128_GCM_SHA256', 'TLS_AES_256_GCM_SHA384', 'TLS_CHACHA20_POLY1305_SHA256']) 586``` 587 588When using [`GitLab::HTTP`](#gitlab-http-library), the code looks like: 589 590This is the **recommended** implementation to avoid security issues such as SSRF: 591 592```ruby 593response = GitLab::HTTP.perform_request(Net::HTTP::Get, 'https://gitlab.com', ssl_version: :TLSv1_3, ciphers: ['TLS_AES_128_GCM_SHA256', 'TLS_AES_256_GCM_SHA384', 'TLS_CHACHA20_POLY1305_SHA256']) 594``` 595 596##### TLS 1.2 597 598**Golang** does support multiple cipher suites that we do not want to use with TLS 1.2. We need to explicitly list authorized ciphers: 599 600```golang 601func secureCipherSuites() []uint16 { 602 return []uint16{ 603 tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, 604 tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, 605 tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, 606 tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, 607 tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305, 608 tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305, 609 } 610``` 611 612And then use `secureCipherSuites()` in `tls.Config`: 613 614```golang 615tls.Config{ 616 (...), 617 CipherSuites: secureCipherSuites(), 618 MinVersion: tls.VersionTLS12, 619 (...), 620} 621``` 622 623This example was taken [here](https://gitlab.com/gitlab-org/cluster-integration/gitlab-agent/-/blob/871b52dc700f1a66f6644fbb1e78a6d463a6ff83/internal/tool/tlstool/tlstool.go#L72). 624 625For **Ruby**, you can use again [HTTParty](https://github.com/jnunemaker/httparty) and specify this time TLS 1.2 version alongside with the recommended ciphers: 626 627```ruby 628response = GitLab::HTTP.perform_request(Net::HTTP::Get, 'https://gitlab.com', ssl_version: :TLSv1_2, ciphers: ['ECDHE-ECDSA-AES128-GCM-SHA256', 'ECDHE-RSA-AES128-GCM-SHA256', 'ECDHE-ECDSA-AES256-GCM-SHA384', 'ECDHE-RSA-AES256-GCM-SHA384', 'ECDHE-ECDSA-CHACHA20-POLY1305', 'ECDHE-RSA-CHACHA20-POLY1305']) 629``` 630 631## GitLab Internal Authorization 632 633### Introduction 634 635There are some cases where `users` passed in the code is actually referring to a `DeployToken`/`DeployKey` entity instead of a real `User`, because of the code below in **`/lib/api/api_guard.rb`** 636 637```ruby 638 def find_user_from_sources 639 strong_memoize(:find_user_from_sources) do 640 deploy_token_from_request || 641 find_user_from_bearer_token || 642 find_user_from_job_token || 643 user_from_warden 644 end 645 end 646``` 647 648### Past Vulnerable Code 649 650In some scenarios such as [this one](https://gitlab.com/gitlab-org/gitlab/-/issues/237795), user impersonation is possible because a `DeployToken` ID can be used in place of a `User` ID. This happened because there was no check on the line with `Gitlab::Auth::CurrentUserMode.bypass_session!(user.id)`. In this case, the `id` is actually a `DeployToken` ID instead of a `User` ID. 651 652```ruby 653 def find_current_user! 654 user = find_user_from_sources 655 return unless user 656 657 # Sessions are enforced to be unavailable for API calls, so ignore them for admin mode 658 Gitlab::Auth::CurrentUserMode.bypass_session!(user.id) if Gitlab::CurrentSettings.admin_mode 659 660 unless api_access_allowed?(user) 661 forbidden!(api_access_denied_message(user)) 662 end 663``` 664 665### Best Practices 666 667In order to prevent this from happening, it is recommended to use the method `user.is_a?(User)` to make sure it returns `true` when we are expecting to deal with a `User` object. This could prevent the ID confusion from the method `find_user_from_sources` mentioned above. Below code snippet shows the fixed code after applying the best practice to the vulnerable code above. 668 669```ruby 670 def find_current_user! 671 user = find_user_from_sources 672 return unless user 673 674 if user.is_a?(User) && Gitlab::CurrentSettings.admin_mode 675 # Sessions are enforced to be unavailable for API calls, so ignore them for admin mode 676 Gitlab::Auth::CurrentUserMode.bypass_session!(user.id) 677 end 678 679 unless api_access_allowed?(user) 680 forbidden!(api_access_denied_message(user)) 681 end 682``` 683 684## Guidelines when defining missing methods with metaprogramming 685 686Metaprogramming is a way to define methods **at runtime**, instead of at the time of writing and deploying the code. It is a powerful tool, but can be dangerous if we allow untrusted actors (like users) to define their own arbitrary methods. For example, imagine we accidentally let an attacker overwrite an access control method to always return true! It can lead to many classes of vulnerabilities such as access control bypass, information disclosure, arbitrary file reads, and remote code execution. 687 688Key methods to watch out for are `method_missing`, `define_method`, `delegate`, and similar methods. 689 690### Insecure metaprogramming example 691 692This example is adapted from an example submitted by [@jobert](https://hackerone.com/jobert?type=user) through our HackerOne bug bounty program. 693Thank you for your contribution! 694 695Before Ruby 2.5.1, you could implement delegators using the `delegate` or `method_missing` methods. For example: 696 697```ruby 698class User 699 def initialize(attributes) 700 @options = OpenStruct.new(attributes) 701 end 702 703 def is_admin? 704 name.eql?("Sid") # Note - never do this! 705 end 706 707 def method_missing(method, *args) 708 @options.send(method, *args) 709 end 710end 711``` 712 713When a method was called on a `User` instance that didn't exist, it passed it along to the `@options` instance variable. 714 715```ruby 716User.new({name: "Jeeves"}).is_admin? 717# => false 718 719User.new(name: "Sid").is_admin? 720# => true 721 722User.new(name: "Jeeves", "is_admin?" => true).is_admin? 723# => false 724``` 725 726Because the `is_admin?` method is already defined on the class, its behavior is not overridden when passing `is_admin?` to the initializer. 727 728This class can be refactored to use the `Forwardable` method and `def_delegators`: 729 730```ruby 731class User 732 extend Forwardable 733 734 def initialize(attributes) 735 @options = OpenStruct.new(attributes) 736 737 self.class.instance_eval do 738 def_delegators :@options, *attributes.keys 739 end 740 end 741 742 def is_admin? 743 name.eql?("Sid") # Note - never do this! 744 end 745end 746``` 747 748It might seem like this example has the same behavior as the first code example. However, there's one crucial difference: **because the delegators are meta-programmed after the class is loaded, it can overwrite existing methods**: 749 750```ruby 751User.new({name: "Jeeves"}).is_admin? 752# => false 753 754User.new(name: "Sid").is_admin? 755# => true 756 757User.new(name: "Jeeves", "is_admin?" => true).is_admin? 758# => true 759# ^------------------ The method is overwritten! Sneaky Jeeves! 760``` 761 762In the example above, the `is_admin?` method is overwritten when passing it to the initializer. 763 764### Best practices 765 766- Never pass user-provided details into method-defining metaprogramming methods. 767 - If you must, be **very** confident that you've sanitized the values correctly. 768 Consider creating an allowlist of values, and validating the user input against that. 769- When extending classes that use metaprogramming, make sure you don't inadvertently override any method definition safety checks. 770