Commit e0e35e0b authored by Nick Malcolm's avatar Nick Malcolm Committed by Nick Gaskill

Secure Code Guidelines: warn about .start_with? for URLs

parent e6a8b5b9
...@@ -267,10 +267,13 @@ of these guidelines. ...@@ -267,10 +267,13 @@ of these guidelines.
#### Feature-specific mitigations #### Feature-specific mitigations
For 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.
There 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. There 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.
For situations in which you can't use an allowlist or GitLab:HTTP, you must implement mitigations
directly in the feature. It's best to validate the destination IP addresses themselves, not just
domain names, as the attacker can control DNS. Below is a list of mitigations that you should
implement.
- Block connections to all localhost addresses - Block connections to all localhost addresses
- `127.0.0.1/8` (IPv4 - note the subnet mask) - `127.0.0.1/8` (IPv4 - note the subnet mask)
- `::1` (IPv6) - `::1` (IPv6)
...@@ -286,6 +289,33 @@ There are many tricks to bypass common SSRF validations. If feature-specific mit ...@@ -286,6 +289,33 @@ There are many tricks to bypass common SSRF validations. If feature-specific mit
See [`url_blocker_spec.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/spec/lib/gitlab/url_blocker_spec.rb) for examples of SSRF payloads. See [time of check to time of use bugs](#time-of-check-to-time-of-use-bugs) to learn more about DNS rebinding's class of bug. See [`url_blocker_spec.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/spec/lib/gitlab/url_blocker_spec.rb) for examples of SSRF payloads. See [time of check to time of use bugs](#time-of-check-to-time-of-use-bugs) to learn more about DNS rebinding's class of bug.
Don't rely on methods like `.start_with?` when validating a URL, or make assumptions about which
part of a string maps to which part of a URL. Use the `URI` class to parse the string, and validate
each component (scheme, host, port, path, and so on). Attackers can create valid URLs which look
safe, but lead to malicious locations.
```ruby
user_supplied_url = "https://my-safe-site.com@my-evil-site.com" # Content before an @ in a URL is usually for basic authentication
user_supplied_url.start_with?("https://my-safe-site.com") # Don't trust with start_with? for URLs!
=> true
URI.parse(user_supplied_url).host
=> "my-evil-site.com"
user_supplied_url = "https://my-safe-site.com-my-evil-site.com"
user_supplied_url.start_with?("https://my-safe-site.com") # Don't trust with start_with? for URLs!
=> true
URI.parse(user_supplied_url).host
=> "my-safe-site.com-my-evil-site.com"
# Here's an example where we unsafely attempt to validate a host while allowing for
# subdomains
user_supplied_url = "https://my-evil-site-my-safe-site.com"
user_supplied_host = URI.parse(user_supplied_url).host
=> "my-evil-site-my-safe-site.com"
user_supplied_host.end_with?("my-safe-site.com") # Don't trust with end_with?
=> true
```
## XSS guidelines ## XSS guidelines
### Description ### Description
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment