From 3c49c4ee357c1d81a4dd1c52ed51e0b70231f3e8 Mon Sep 17 00:00:00 2001
From: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Date: Wed, 8 Mar 2023 07:16:51 +0800
Subject: [PATCH] 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.
---
 lib/freedom_patches/web_push_request.rb   | 27 ++++++++++
 spec/lib/freedom_patches/web_push_spec.rb | 60 +++++++++++++++++++++++
 2 files changed, 87 insertions(+)
 create mode 100644 lib/freedom_patches/web_push_request.rb
 create mode 100644 spec/lib/freedom_patches/web_push_spec.rb

diff --git a/lib/freedom_patches/web_push_request.rb b/lib/freedom_patches/web_push_request.rb
new file mode 100644
index 00000000000..ee6c23ce477
--- /dev/null
+++ b/lib/freedom_patches/web_push_request.rb
@@ -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)
diff --git a/spec/lib/freedom_patches/web_push_spec.rb b/spec/lib/freedom_patches/web_push_spec.rb
new file mode 100644
index 00000000000..d5164258374
--- /dev/null
+++ b/spec/lib/freedom_patches/web_push_spec.rb
@@ -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