Skip to content

Commit 496851b

Browse files
committed
Security: Web Push SSRF and IP range bypass
Add SSRF protection for web push endpoints: - Resolve endpoint IP once and pin it for connection - Validate endpoints resolve to public IPs - Whitelist permitted push service hosts Add missing IP ranges to SsrfProtection: - 100.64.0.0/10 (Carrier-grade NAT, RFC6598) - 198.18.0.0/15 (Benchmark testing, RFC2544) Note: link-local (169.254.0.0/16) is already covered by ip.link_local?
1 parent fee376b commit 496851b

File tree

7 files changed

+276
-8
lines changed

7 files changed

+276
-8
lines changed

app/models/push/subscription.rb

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,54 @@
11
class Push::Subscription < ApplicationRecord
2+
PERMITTED_ENDPOINT_HOSTS = %w[
3+
fcm.googleapis.com
4+
updates.push.services.mozilla.com
5+
web.push.apple.com
6+
notify.windows.com
7+
].freeze
8+
29
belongs_to :account, default: -> { user.account }
310
belongs_to :user
411

12+
validates :endpoint, presence: true
13+
validate :validate_endpoint_url
14+
515
def notification(**params)
616
WebPush::Notification.new(
717
**params,
818
badge: user.notifications.unread.count,
919
endpoint: endpoint,
20+
endpoint_ip: resolved_endpoint_ip,
1021
p256dh_key: p256dh_key,
1122
auth_key: auth_key
1223
)
1324
end
25+
26+
def resolved_endpoint_ip
27+
return @resolved_endpoint_ip if defined?(@resolved_endpoint_ip)
28+
@resolved_endpoint_ip = SsrfProtection.resolve_public_ip(endpoint_uri&.host)
29+
end
30+
31+
private
32+
def endpoint_uri
33+
@endpoint_uri ||= URI.parse(endpoint) if endpoint.present?
34+
rescue URI::InvalidURIError
35+
nil
36+
end
37+
38+
def validate_endpoint_url
39+
if endpoint_uri.nil?
40+
errors.add(:endpoint, "is not a valid URL")
41+
elsif endpoint_uri.scheme != "https"
42+
errors.add(:endpoint, "must use HTTPS")
43+
elsif !permitted_endpoint_host?
44+
errors.add(:endpoint, "is not a permitted push service")
45+
elsif resolved_endpoint_ip.nil?
46+
errors.add(:endpoint, "resolves to a private or invalid IP address")
47+
end
48+
end
49+
50+
def permitted_endpoint_host?
51+
host = endpoint_uri&.host&.downcase
52+
PERMITTED_ENDPOINT_HOSTS.any? { |permitted| host&.end_with?(permitted) }
53+
end
1454
end

app/models/ssrf_protection.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ module SsrfProtection
44
DNS_RESOLUTION_TIMEOUT = 2
55

66
DISALLOWED_IP_RANGES = [
7-
IPAddr.new("0.0.0.0/8") # Broadcasts
7+
IPAddr.new("0.0.0.0/8"), # "This" network (RFC1700)
8+
IPAddr.new("100.64.0.0/10"), # Carrier-grade NAT (RFC6598)
9+
IPAddr.new("198.18.0.0/15") # Benchmark testing (RFC2544)
810
].freeze
911

1012
def resolve_public_ip(hostname)

config/initializers/web_push.rb

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,15 @@
1717

1818
module WebPush::PersistentRequest
1919
def perform
20-
if @options[:connection]
20+
endpoint_ip = @options[:endpoint_ip]
21+
22+
if endpoint_ip
23+
http = Net::HTTP.new(uri.host, uri.port, ipaddr: endpoint_ip)
24+
http.use_ssl = true
25+
http.ssl_timeout = @options[:ssl_timeout] unless @options[:ssl_timeout].nil?
26+
http.open_timeout = @options[:open_timeout] unless @options[:open_timeout].nil?
27+
http.read_timeout = @options[:read_timeout] unless @options[:read_timeout].nil?
28+
elsif @options[:connection]
2129
http = @options[:connection]
2230
else
2331
http = Net::HTTP.new(uri.host, uri.port, *proxy_options)

lib/web_push/notification.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
class WebPush::Notification
2-
def initialize(title:, body:, path:, badge:, endpoint:, p256dh_key:, auth_key:)
2+
def initialize(title:, body:, path:, badge:, endpoint:, endpoint_ip:, p256dh_key:, auth_key:)
33
@title, @body, @path, @badge = title, body, path, badge
4-
@endpoint, @p256dh_key, @auth_key = endpoint, p256dh_key, auth_key
4+
@endpoint, @endpoint_ip, @p256dh_key, @auth_key = endpoint, endpoint_ip, p256dh_key, auth_key
55
end
66

77
def deliver(connection: nil)
88
WebPush.payload_send \
99
message: encoded_message,
10-
endpoint: @endpoint, p256dh: @p256dh_key, auth: @auth_key,
10+
endpoint: @endpoint, endpoint_ip: @endpoint_ip, p256dh: @p256dh_key, auth: @auth_key,
1111
vapid: vapid_identification,
1212
connection: connection,
1313
urgency: "high"

test/controllers/users/push_subscriptions_controller_test.rb

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
require "test_helper"
22

33
class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest
4+
PUBLIC_TEST_IP = "142.250.185.206"
5+
46
setup do
57
sign_in_as :david
8+
stub_dns_resolution(PUBLIC_TEST_IP)
69
end
710

811
test "create new push subscription" do
9-
subscription_params = { "endpoint" => "https://apple", "p256dh_key" => "123", "auth_key" => "456" }
12+
subscription_params = { "endpoint" => "https://fcm.googleapis.com/fcm/send/abc123", "p256dh_key" => "123", "auth_key" => "456" }
1013

1114
post user_push_subscriptions_path(users(:david)),
1215
params: { push_subscription: subscription_params }, headers: { "HTTP_USER_AGENT" => "Mozilla/5.0" }
@@ -19,7 +22,7 @@ class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest
1922

2023
test "touch existing subscription" do
2124
existing_subscription = users(:david).push_subscriptions.create!(
22-
endpoint: "https://apple",
25+
endpoint: "https://fcm.googleapis.com/fcm/send/abc123",
2326
p256dh_key: "123",
2427
auth_key: "456"
2528
)
@@ -37,7 +40,7 @@ class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest
3740

3841
test "destroy a push subscription" do
3942
subscription = users(:david).push_subscriptions.create!(
40-
endpoint: "https://apple",
43+
endpoint: "https://fcm.googleapis.com/fcm/send/abc123",
4144
p256dh_key: "123",
4245
auth_key: "456"
4346
)
@@ -47,4 +50,35 @@ class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest
4750
assert_redirected_to user_push_subscriptions_path(users(:david))
4851
end
4952
end
53+
54+
test "rejects subscription with non-permitted endpoint" do
55+
subscription_params = { "endpoint" => "https://attacker.example.com/steal", "p256dh_key" => "123", "auth_key" => "456" }
56+
57+
assert_no_difference -> { Push::Subscription.count } do
58+
post user_push_subscriptions_path(users(:david)),
59+
params: { push_subscription: subscription_params }
60+
end
61+
62+
assert_response :unprocessable_entity
63+
end
64+
65+
test "rejects subscription with endpoint resolving to private IP" do
66+
stub_dns_resolution("192.168.1.1")
67+
68+
subscription_params = { "endpoint" => "https://fcm.googleapis.com/fcm/send/abc123", "p256dh_key" => "123", "auth_key" => "456" }
69+
70+
assert_no_difference -> { Push::Subscription.count } do
71+
post user_push_subscriptions_path(users(:david)),
72+
params: { push_subscription: subscription_params }
73+
end
74+
75+
assert_response :unprocessable_entity
76+
end
77+
78+
private
79+
def stub_dns_resolution(*ips)
80+
dns_mock = mock("dns")
81+
dns_mock.stubs(:each_address).multiple_yields(*ips)
82+
Resolv::DNS.stubs(:open).yields(dns_mock)
83+
end
5084
end
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
require "test_helper"
2+
3+
class Push::SubscriptionTest < ActiveSupport::TestCase
4+
PUBLIC_TEST_IP = "142.250.185.206" # google.com IP
5+
6+
setup do
7+
stub_dns_resolution(PUBLIC_TEST_IP)
8+
end
9+
10+
test "valid subscription with permitted endpoint" do
11+
subscription = Push::Subscription.new(
12+
user: users(:david),
13+
endpoint: "https://fcm.googleapis.com/fcm/send/abc123",
14+
p256dh_key: "test_key",
15+
auth_key: "test_auth"
16+
)
17+
18+
assert subscription.valid?
19+
end
20+
21+
test "rejects endpoint with non-https scheme" do
22+
subscription = Push::Subscription.new(
23+
user: users(:david),
24+
endpoint: "http://fcm.googleapis.com/fcm/send/abc123",
25+
p256dh_key: "test_key",
26+
auth_key: "test_auth"
27+
)
28+
29+
assert_not subscription.valid?
30+
assert_includes subscription.errors[:endpoint], "must use HTTPS"
31+
end
32+
33+
test "rejects endpoint with non-permitted host" do
34+
subscription = Push::Subscription.new(
35+
user: users(:david),
36+
endpoint: "https://attacker.example.com/webhook",
37+
p256dh_key: "test_key",
38+
auth_key: "test_auth"
39+
)
40+
41+
assert_not subscription.valid?
42+
assert_includes subscription.errors[:endpoint], "is not a permitted push service"
43+
end
44+
45+
test "rejects endpoint that resolves to private IP" do
46+
stub_dns_resolution("192.168.1.1")
47+
48+
subscription = Push::Subscription.new(
49+
user: users(:david),
50+
endpoint: "https://fcm.googleapis.com/fcm/send/abc123",
51+
p256dh_key: "test_key",
52+
auth_key: "test_auth"
53+
)
54+
55+
assert_not subscription.valid?
56+
assert_includes subscription.errors[:endpoint], "resolves to a private or invalid IP address"
57+
end
58+
59+
test "rejects endpoint that resolves to loopback IP" do
60+
stub_dns_resolution("127.0.0.1")
61+
62+
subscription = Push::Subscription.new(
63+
user: users(:david),
64+
endpoint: "https://fcm.googleapis.com/fcm/send/abc123",
65+
p256dh_key: "test_key",
66+
auth_key: "test_auth"
67+
)
68+
69+
assert_not subscription.valid?
70+
assert_includes subscription.errors[:endpoint], "resolves to a private or invalid IP address"
71+
end
72+
73+
test "rejects endpoint that resolves to link-local IP (AWS IMDS)" do
74+
stub_dns_resolution("169.254.169.254")
75+
76+
subscription = Push::Subscription.new(
77+
user: users(:david),
78+
endpoint: "https://fcm.googleapis.com/fcm/send/abc123",
79+
p256dh_key: "test_key",
80+
auth_key: "test_auth"
81+
)
82+
83+
assert_not subscription.valid?
84+
assert_includes subscription.errors[:endpoint], "resolves to a private or invalid IP address"
85+
end
86+
87+
test "resolved_endpoint_ip returns pinned public IP" do
88+
subscription = Push::Subscription.new(
89+
user: users(:david),
90+
endpoint: "https://fcm.googleapis.com/fcm/send/abc123",
91+
p256dh_key: "test_key",
92+
auth_key: "test_auth"
93+
)
94+
95+
assert_equal PUBLIC_TEST_IP, subscription.resolved_endpoint_ip
96+
end
97+
98+
test "accepts all permitted push service domains" do
99+
permitted_endpoints = [
100+
"https://fcm.googleapis.com/fcm/send/token123",
101+
"https://updates.push.services.mozilla.com/wpush/v2/token123",
102+
"https://web.push.apple.com/QaBC123",
103+
"https://wns2-db5p.notify.windows.com/w/?token=abc123"
104+
]
105+
106+
permitted_endpoints.each do |endpoint|
107+
subscription = Push::Subscription.new(
108+
user: users(:david),
109+
endpoint: endpoint,
110+
p256dh_key: "test_key",
111+
auth_key: "test_auth"
112+
)
113+
114+
assert subscription.valid?, "Expected #{endpoint} to be valid, got errors: #{subscription.errors.full_messages}"
115+
end
116+
end
117+
118+
private
119+
def stub_dns_resolution(*ips)
120+
dns_mock = mock("dns")
121+
dns_mock.stubs(:each_address).multiple_yields(*ips)
122+
Resolv::DNS.stubs(:open).yields(dns_mock)
123+
end
124+
end
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
require "test_helper"
2+
3+
class SsrfProtectionTest < ActiveSupport::TestCase
4+
test "blocks loopback addresses" do
5+
stub_dns_resolution("127.0.0.1")
6+
assert_nil SsrfProtection.resolve_public_ip("localhost")
7+
end
8+
9+
test "blocks private 10.x.x.x addresses" do
10+
stub_dns_resolution("10.0.0.1")
11+
assert_nil SsrfProtection.resolve_public_ip("internal.example.com")
12+
end
13+
14+
test "blocks private 172.16.x.x addresses" do
15+
stub_dns_resolution("172.16.0.1")
16+
assert_nil SsrfProtection.resolve_public_ip("internal.example.com")
17+
end
18+
19+
test "blocks private 192.168.x.x addresses" do
20+
stub_dns_resolution("192.168.1.1")
21+
assert_nil SsrfProtection.resolve_public_ip("internal.example.com")
22+
end
23+
24+
test "blocks link-local addresses (AWS metadata endpoint)" do
25+
stub_dns_resolution("169.254.169.254")
26+
assert_nil SsrfProtection.resolve_public_ip("metadata.example.com")
27+
end
28+
29+
test "blocks carrier-grade NAT addresses" do
30+
stub_dns_resolution("100.64.0.1")
31+
assert_nil SsrfProtection.resolve_public_ip("cgnat.example.com")
32+
end
33+
34+
test "blocks benchmark testing addresses" do
35+
stub_dns_resolution("198.18.0.1")
36+
assert_nil SsrfProtection.resolve_public_ip("benchmark.example.com")
37+
end
38+
39+
test "blocks broadcast addresses" do
40+
stub_dns_resolution("0.0.0.1")
41+
assert_nil SsrfProtection.resolve_public_ip("broadcast.example.com")
42+
end
43+
44+
test "allows public addresses" do
45+
stub_dns_resolution("93.184.216.34")
46+
assert_equal "93.184.216.34", SsrfProtection.resolve_public_ip("example.com")
47+
end
48+
49+
test "returns first public IP when multiple addresses resolve" do
50+
stub_dns_resolution("10.0.0.1", "93.184.216.34", "192.168.1.1")
51+
assert_equal "93.184.216.34", SsrfProtection.resolve_public_ip("multi.example.com")
52+
end
53+
54+
private
55+
def stub_dns_resolution(*ips)
56+
dns_mock = mock("dns")
57+
dns_mock.stubs(:each_address).multiple_yields(*ips)
58+
Resolv::DNS.stubs(:open).yields(dns_mock)
59+
end
60+
end

0 commit comments

Comments
 (0)