bterkuile/fizzy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #1905 from basecamp/web-push-ssrf

Browse Source 
Security: Web Push SSRF
This commit is contained in:
Jorge Manrubia
2025-12-07 11:58:14 +01:00
committed by GitHub
parent 91017c9208 496851b255
commit 568783efd1
7 changed files with 276 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/models/push/subscription.rb
+40
View File
@@ -1,14 +1,54 @@
class Push::Subscription < ApplicationRecord
PERMITTED_ENDPOINT_HOSTS = %w[
fcm.googleapis.com
updates.push.services.mozilla.com
web.push.apple.com
notify.windows.com
].freeze
belongs_to :account, default: -> { user.account }
belongs_to :user
validates :endpoint, presence: true
validate :validate_endpoint_url
def notification(**params)
WebPush::Notification.new(
**params,
badge: user.notifications.unread.count,
endpoint: endpoint,
endpoint_ip: resolved_endpoint_ip,
p256dh_key: p256dh_key,
auth_key: auth_key
)
end
def resolved_endpoint_ip
return @resolved_endpoint_ip if defined?(@resolved_endpoint_ip)
@resolved_endpoint_ip = SsrfProtection.resolve_public_ip(endpoint_uri&.host)
end
private
def endpoint_uri
@endpoint_uri ||= URI.parse(endpoint) if endpoint.present?
rescue URI::InvalidURIError
nil
end
def validate_endpoint_url
if endpoint_uri.nil?
errors.add(:endpoint, "is not a valid URL")
elsif endpoint_uri.scheme != "https"
errors.add(:endpoint, "must use HTTPS")
elsif !permitted_endpoint_host?
errors.add(:endpoint, "is not a permitted push service")
elsif resolved_endpoint_ip.nil?
errors.add(:endpoint, "resolves to a private or invalid IP address")
end
end
def permitted_endpoint_host?
host = endpoint_uri&.host&.downcase
PERMITTED_ENDPOINT_HOSTS.any? { |permitted| host&.end_with?(permitted) }
end
end
app/models/ssrf_protection.rb
+3 -1
View File
@@ -9,7 +9,9 @@ module SsrfProtection
]
DISALLOWED_IP_RANGES = [
IPAddr.new("0.0.0.0/8") # Broadcasts
IPAddr.new("0.0.0.0/8"), # "This" network (RFC1700)
IPAddr.new("100.64.0.0/10"), # Carrier-grade NAT (RFC6598)
IPAddr.new("198.18.0.0/15") # Benchmark testing (RFC2544)
].freeze
def resolve_public_ip(hostname)
config/initializers/web_push.rb
+9 -1
View File
@@ -17,7 +17,15 @@ end
module WebPush::PersistentRequest
def perform
if @options[:connection]
endpoint_ip = @options[:endpoint_ip]
if endpoint_ip
http = Net::HTTP.new(uri.host, uri.port, ipaddr: endpoint_ip)
http.use_ssl = true
http.ssl_timeout = @options[:ssl_timeout] unless @options[:ssl_timeout].nil?
http.open_timeout = @options[:open_timeout] unless @options[:open_timeout].nil?
http.read_timeout = @options[:read_timeout] unless @options[:read_timeout].nil?
elsif @options[:connection]
http = @options[:connection]
else
http = Net::HTTP.new(uri.host, uri.port, *proxy_options)
lib/web_push/notification.rb
+3 -3
View File
@@ -1,13 +1,13 @@
class WebPush::Notification
def initialize(title:, body:, path:, badge:, endpoint:, p256dh_key:, auth_key:)
def initialize(title:, body:, path:, badge:, endpoint:, endpoint_ip:, p256dh_key:, auth_key:)
@title, @body, @path, @badge = title, body, path, badge
@endpoint, @p256dh_key, @auth_key = endpoint, p256dh_key, auth_key
@endpoint, @endpoint_ip, @p256dh_key, @auth_key = endpoint, endpoint_ip, p256dh_key, auth_key
end
def deliver(connection: nil)
WebPush.payload_send \
message: encoded_message,
endpoint: @endpoint, p256dh: @p256dh_key, auth: @auth_key,
endpoint: @endpoint, endpoint_ip: @endpoint_ip, p256dh: @p256dh_key, auth: @auth_key,
vapid: vapid_identification,
connection: connection,
urgency: "high"
test/controllers/users/push_subscriptions_controller_test.rb
+37 -3
View File
@@ -1,12 +1,15 @@
require "test_helper"
class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest
PUBLIC_TEST_IP = "142.250.185.206"
setup do
sign_in_as :david
stub_dns_resolution(PUBLIC_TEST_IP)
end
test "create new push subscription" do
subscription_params = { "endpoint" => "https://apple", "p256dh_key" => "123", "auth_key" => "456" }
subscription_params = { "endpoint" => "https://fcm.googleapis.com/fcm/send/abc123", "p256dh_key" => "123", "auth_key" => "456" }
post user_push_subscriptions_path(users(:david)),
params: { push_subscription: subscription_params }, headers: { "HTTP_USER_AGENT" => "Mozilla/5.0" }
@@ -19,7 +22,7 @@ class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest
test "touch existing subscription" do
existing_subscription = users(:david).push_subscriptions.create!(
endpoint: "https://apple",
endpoint: "https://fcm.googleapis.com/fcm/send/abc123",
p256dh_key: "123",
auth_key: "456"
)
@@ -37,7 +40,7 @@ class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest
test "destroy a push subscription" do
subscription = users(:david).push_subscriptions.create!(
endpoint: "https://apple",
endpoint: "https://fcm.googleapis.com/fcm/send/abc123",
p256dh_key: "123",
auth_key: "456"
)
@@ -47,4 +50,35 @@ class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest
assert_redirected_to user_push_subscriptions_path(users(:david))
end
end
test "rejects subscription with non-permitted endpoint" do
subscription_params = { "endpoint" => "https://attacker.example.com/steal", "p256dh_key" => "123", "auth_key" => "456" }
assert_no_difference -> { Push::Subscription.count } do
post user_push_subscriptions_path(users(:david)),
params: { push_subscription: subscription_params }
end
assert_response :unprocessable_entity
end
test "rejects subscription with endpoint resolving to private IP" do
stub_dns_resolution("192.168.1.1")
subscription_params = { "endpoint" => "https://fcm.googleapis.com/fcm/send/abc123", "p256dh_key" => "123", "auth_key" => "456" }
assert_no_difference -> { Push::Subscription.count } do
post user_push_subscriptions_path(users(:david)),
params: { push_subscription: subscription_params }
end
assert_response :unprocessable_entity
end
private
def stub_dns_resolution(*ips)
dns_mock = mock("dns")
dns_mock.stubs(:each_address).multiple_yields(*ips)
Resolv::DNS.stubs(:open).yields(dns_mock)
end
end
test/models/push/subscription_test.rb
+124
View File
@@ -0,0 +1,124 @@
require "test_helper"
class Push::SubscriptionTest < ActiveSupport::TestCase
PUBLIC_TEST_IP = "142.250.185.206" # google.com IP
setup do
stub_dns_resolution(PUBLIC_TEST_IP)
end
test "valid subscription with permitted endpoint" do
subscription = Push::Subscription.new(
user: users(:david),
endpoint: "https://fcm.googleapis.com/fcm/send/abc123",
p256dh_key: "test_key",
auth_key: "test_auth"
)
assert subscription.valid?
end
test "rejects endpoint with non-https scheme" do
subscription = Push::Subscription.new(
user: users(:david),
endpoint: "http://fcm.googleapis.com/fcm/send/abc123",
p256dh_key: "test_key",
auth_key: "test_auth"
)
assert_not subscription.valid?
assert_includes subscription.errors[:endpoint], "must use HTTPS"
end
test "rejects endpoint with non-permitted host" do
subscription = Push::Subscription.new(
user: users(:david),
endpoint: "https://attacker.example.com/webhook",
p256dh_key: "test_key",
auth_key: "test_auth"
)
assert_not subscription.valid?
assert_includes subscription.errors[:endpoint], "is not a permitted push service"
end
test "rejects endpoint that resolves to private IP" do
stub_dns_resolution("192.168.1.1")
subscription = Push::Subscription.new(
user: users(:david),
endpoint: "https://fcm.googleapis.com/fcm/send/abc123",
p256dh_key: "test_key",
auth_key: "test_auth"
)
assert_not subscription.valid?
assert_includes subscription.errors[:endpoint], "resolves to a private or invalid IP address"
end
test "rejects endpoint that resolves to loopback IP" do
stub_dns_resolution("127.0.0.1")
subscription = Push::Subscription.new(
user: users(:david),
endpoint: "https://fcm.googleapis.com/fcm/send/abc123",
p256dh_key: "test_key",
auth_key: "test_auth"
)
assert_not subscription.valid?
assert_includes subscription.errors[:endpoint], "resolves to a private or invalid IP address"
end
test "rejects endpoint that resolves to link-local IP (AWS IMDS)" do
stub_dns_resolution("169.254.169.254")
subscription = Push::Subscription.new(
user: users(:david),
endpoint: "https://fcm.googleapis.com/fcm/send/abc123",
p256dh_key: "test_key",
auth_key: "test_auth"
)
assert_not subscription.valid?
assert_includes subscription.errors[:endpoint], "resolves to a private or invalid IP address"
end
test "resolved_endpoint_ip returns pinned public IP" do
subscription = Push::Subscription.new(
user: users(:david),
endpoint: "https://fcm.googleapis.com/fcm/send/abc123",
p256dh_key: "test_key",
auth_key: "test_auth"
)
assert_equal PUBLIC_TEST_IP, subscription.resolved_endpoint_ip
end
test "accepts all permitted push service domains" do
permitted_endpoints = [
"https://fcm.googleapis.com/fcm/send/token123",
"https://updates.push.services.mozilla.com/wpush/v2/token123",
"https://web.push.apple.com/QaBC123",
"https://wns2-db5p.notify.windows.com/w/?token=abc123"
]
permitted_endpoints.each do |endpoint|
subscription = Push::Subscription.new(
user: users(:david),
endpoint: endpoint,
p256dh_key: "test_key",
auth_key: "test_auth"
)
assert subscription.valid?, "Expected #{endpoint} to be valid, got errors: #{subscription.errors.full_messages}"
end
end
private
def stub_dns_resolution(*ips)
dns_mock = mock("dns")
dns_mock.stubs(:each_address).multiple_yields(*ips)
Resolv::DNS.stubs(:open).yields(dns_mock)
end
end
test/models/ssrf_protection_test.rb
+60
View File
@@ -0,0 +1,60 @@
require "test_helper"
class SsrfProtectionTest < ActiveSupport::TestCase
test "blocks loopback addresses" do
stub_dns_resolution("127.0.0.1")
assert_nil SsrfProtection.resolve_public_ip("localhost")
end
test "blocks private 10.x.x.x addresses" do
stub_dns_resolution("10.0.0.1")
assert_nil SsrfProtection.resolve_public_ip("internal.example.com")
end
test "blocks private 172.16.x.x addresses" do
stub_dns_resolution("172.16.0.1")
assert_nil SsrfProtection.resolve_public_ip("internal.example.com")
end
test "blocks private 192.168.x.x addresses" do
stub_dns_resolution("192.168.1.1")
assert_nil SsrfProtection.resolve_public_ip("internal.example.com")
end
test "blocks link-local addresses (AWS metadata endpoint)" do
stub_dns_resolution("169.254.169.254")
assert_nil SsrfProtection.resolve_public_ip("metadata.example.com")
end
test "blocks carrier-grade NAT addresses" do
stub_dns_resolution("100.64.0.1")
assert_nil SsrfProtection.resolve_public_ip("cgnat.example.com")
end
test "blocks benchmark testing addresses" do
stub_dns_resolution("198.18.0.1")
assert_nil SsrfProtection.resolve_public_ip("benchmark.example.com")
end
test "blocks broadcast addresses" do
stub_dns_resolution("0.0.0.1")
assert_nil SsrfProtection.resolve_public_ip("broadcast.example.com")
end
test "allows public addresses" do
stub_dns_resolution("93.184.216.34")
assert_equal "93.184.216.34", SsrfProtection.resolve_public_ip("example.com")
end
test "returns first public IP when multiple addresses resolve" do
stub_dns_resolution("10.0.0.1", "93.184.216.34", "192.168.1.1")
assert_equal "93.184.216.34", SsrfProtection.resolve_public_ip("multi.example.com")
end
private
def stub_dns_resolution(*ips)
dns_mock = mock("dns")
dns_mock.stubs(:each_address).multiple_yields(*ips)
Resolv::DNS.stubs(:open).yields(dns_mock)
end
end