Guard against SSRF attacks in link verification

Uses bhuga/faraday-restrict-ip-addresses#16 Signed-off-by: Samuel Giddins <[email protected]>
rubygems · Sep 30, 2024 · 8e05e54 · 8e05e54
1 parent 6b0c32d
commit 8e05e54
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 4 deletions.
diff --git a/Gemfile b/Gemfile
@@ -15,6 +15,7 @@ gem "dogstatsd-ruby", "~> 5.6"
 gem "google-protobuf", "~> 4.28"
 gem "faraday", "~> 2.12"
 gem "faraday-retry", "~> 2.2"
+gem "faraday-restrict-ip-addresses", "~> 0.3.0"
 gem "good_job", "~> 3.99"
 gem "gravtastic", "~> 3.2"
 gem "honeybadger", "~> 5.5.1", require: false # see https://github.com/rubygems/rubygems.org/pull/4598

diff --git a/Gemfile.lock b/Gemfile.lock
@@ -250,6 +250,8 @@ GEM
       multipart-post (~> 2)
     faraday-net_http (3.3.0)
       net-http
+    faraday-restrict-ip-addresses (0.3.0)
+      faraday (> 1.0, < 3.0)
     faraday-retry (2.2.1)
       faraday (~> 2.0)
     faraday_middleware-aws-sigv4 (1.0.1)
@@ -824,6 +826,7 @@ DEPENDENCIES
   factory_bot_rails (~> 6.4)
   faraday (~> 2.12)
   faraday-multipart (~> 1.0)
+  faraday-restrict-ip-addresses (~> 0.3.0)
   faraday-retry (~> 2.2)
   faraday_middleware-aws-sigv4 (~> 1.0)
   gem_server_conformance (~> 0.1.4)
@@ -1000,6 +1003,7 @@ CHECKSUMS
   faraday-follow_redirects (0.3.0) sha256=d92d975635e2c7fe525dd494fcd4b9bb7f0a4a0ec0d5f4c15c729530fdb807f9
   faraday-multipart (1.0.4) sha256=9012021ab57790f7d712f590b48d5f948b19b43cfa11ca83e6459f06090b0725
   faraday-net_http (3.3.0) sha256=93e6b0f679b1e8e358bcb4e983a52328dfc47ebbe6a232e4f9e8aba9c924e565
+  faraday-restrict-ip-addresses (0.3.0) sha256=2db7f6da6f59fa73a9d4ffe96b2b46a4b2b2c4d177e44a1962c1921d32ec7279
   faraday-retry (2.2.1) sha256=4146fed14549c0580bf14591fca419a40717de0dd24f267a8ec2d9a728677608
   faraday_middleware-aws-sigv4 (1.0.1) sha256=a001ea4f687ca1c60bad8f2a627196905ce3dbf285e461dc153240e92eaabe8f
   ffi (1.17.0) sha256=51630e43425078311c056ca75f961bb3bda1641ab36e44ad4c455e0b0e4a231c

diff --git a/app/jobs/verify_link_job.rb b/app/jobs/verify_link_job.rb
@@ -9,10 +9,6 @@ class NotHTTPSError < StandardError; end
   class LinkNotPresentError < StandardError; end
   class HTTPResponseError < StandardError; end
 
-  rescue_from NotHTTPSError do |_error|
-    record_failure
-  end
-
   rescue_from LinkNotPresentError, HTTPResponseError, *ERRORS do |error|
     logger.info "Linkback verification failed with error: #{error.message}", error: error, uri: link_verification.uri,
       linkable: link_verification.linkable.to_gid
@@ -23,6 +19,10 @@ class HTTPResponseError < StandardError; end
     end
   end
 
+  rescue_from NotHTTPSError, Faraday::RestrictIPAddresses::AddressNotAllowed do |_error|
+    record_failure
+  end
+
   TIMEOUT_SEC = 5
 
   def perform(link_verification:)
@@ -68,6 +68,9 @@ def verify_link!(uri, linkable)
 
   def get(url)
     Faraday.new(nil, request: { timeout: TIMEOUT_SEC }) do |f|
+      # prevent SSRF attacks
+      f.request :restrict_ip_addresses, deny_rfc6890: true
+
       f.response :logger, logger, headers: false, errors: true
       f.response :raise_error
     end.get(

diff --git a/test/jobs/verify_link_job_test.rb b/test/jobs/verify_link_job_test.rb
@@ -199,4 +199,16 @@ class VerifyLinkJobTest < ActiveJob::TestCase
     assert_equal 1, @docs.failures_since_last_verification
     assert_enqueued_jobs 0, only: VerifyLinkJob
   end
+
+  test "does not retry link verification for localhost link" do
+    @home.update!(uri: "https://localhost")
+    freeze_time
+
+    VerifyLinkJob.perform_now(link_verification: @home)
+
+    refute_predicate @home.reload, :verified?
+    assert_nil @home.last_verified_at
+    assert_equal 1, @home.failures_since_last_verification
+    assert_enqueued_jobs 0, only: VerifyLinkJob
+  end
 end