mirror of
https://github.com/discourse/discourse.git
synced 2024-11-25 09:42:07 +08:00
SECURITY: Monkey-patch web-push gem to use safer HTTP client
`FinalDestination::HTTP` is our patch of `Net::HTTP` which defend us against SSRF and DNS rebinding attacks.
This commit is contained in:
parent
d89b537d8f
commit
52ef44f43b
27
lib/freedom_patches/web_push_request.rb
Normal file
27
lib/freedom_patches/web_push_request.rb
Normal file
|
@ -0,0 +1,27 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
# This is a patch to avoid the direct use of `Net::HTTP` in the `webpush` gem and instead rely on `FinalDestination::HTTP`
|
||||
# which protects us from DNS rebinding attacks as well as server side forgery requests.
|
||||
#
|
||||
# This patch is considered temporary until we can decide on a longer term solution. In the meantime, we need to patch
|
||||
# the SSRF vulnerability being exposed by this gem.
|
||||
module WebPushPatch
|
||||
def perform
|
||||
http = FinalDestination::HTTP.new(uri.host, uri.port, *proxy_options)
|
||||
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?
|
||||
|
||||
req = FinalDestination::HTTP::Post.new(uri.request_uri, headers)
|
||||
req.body = body
|
||||
|
||||
resp = http.request(req)
|
||||
verify_response(resp)
|
||||
|
||||
resp
|
||||
end
|
||||
end
|
||||
|
||||
klass = defined?(WebPush) ? WebPush : Webpush
|
||||
klass::Request.prepend(WebPushPatch)
|
60
spec/lib/freedom_patches/web_push_spec.rb
Normal file
60
spec/lib/freedom_patches/web_push_spec.rb
Normal file
|
@ -0,0 +1,60 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
klass = defined?(WebPush) ? WebPush : Webpush
|
||||
|
||||
RSpec.describe klass do
|
||||
before do
|
||||
FinalDestination::SSRFDetector.allow_ip_lookups_in_test!
|
||||
WebMock.enable!(except: [:final_destination])
|
||||
end
|
||||
|
||||
after do
|
||||
WebMock.enable!
|
||||
FinalDestination::SSRFDetector.disallow_ip_lookups_in_test!
|
||||
end
|
||||
|
||||
it "should filter endpoint hostname through our SSRF detector" do
|
||||
klass::Request.any_instance.expects(:encrypt_payload)
|
||||
klass::Request.any_instance.expects(:headers)
|
||||
|
||||
stub_ip_lookup("example.com", %W[0.0.0.0])
|
||||
|
||||
expect do
|
||||
klass.payload_send(
|
||||
endpoint: "http://example.com",
|
||||
message: "test",
|
||||
p256dh: "somep256dh",
|
||||
auth: "someauth",
|
||||
vapid: {
|
||||
subject: "someurl",
|
||||
public_key: "somepublickey",
|
||||
private_key: "someprivatekey",
|
||||
},
|
||||
)
|
||||
end.to raise_error(FinalDestination::SSRFDetector::DisallowedIpError)
|
||||
end
|
||||
|
||||
it "should send the right request if endpoint hostname resolves to a public ip address" do
|
||||
klass::Request.any_instance.expects(:encrypt_payload)
|
||||
klass::Request.any_instance.expects(:headers)
|
||||
|
||||
stub_ip_lookup("example.com", %W[52.125.123.12])
|
||||
|
||||
success = Class.new(StandardError)
|
||||
TCPSocket.stubs(:open).with { |addr| "52.125.123.12" == addr }.once.raises(success)
|
||||
|
||||
expect do
|
||||
klass.payload_send(
|
||||
endpoint: "http://example.com",
|
||||
message: "test",
|
||||
p256dh: "somep256dh",
|
||||
auth: "someauth",
|
||||
vapid: {
|
||||
subject: "someurl",
|
||||
public_key: "somepublickey",
|
||||
private_key: "someprivatekey",
|
||||
},
|
||||
)
|
||||
end.to raise_error(success)
|
||||
end
|
||||
end
|
Loading…
Reference in New Issue
Block a user