mirror of
https://github.com/discourse/discourse.git
synced 2025-02-28 05:34:09 +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
3374457c44
commit
3c49c4ee35
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…
x
Reference in New Issue
Block a user