Skip to content

Commit

Permalink
Guard against SSRF attacks in link verification
Browse files Browse the repository at this point in the history
Uses bhuga/faraday-restrict-ip-addresses#16

Signed-off-by: Samuel Giddins <[email protected]>
  • Loading branch information
segiddins authored and simi committed Sep 30, 2024
1 parent 4ae91b5 commit ea62255
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 4 deletions.
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -869,6 +871,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)
Expand Down Expand Up @@ -1045,6 +1048,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
Expand Down
11 changes: 7 additions & 4 deletions app/jobs/verify_link_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:)
Expand Down Expand Up @@ -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(
Expand Down
12 changes: 12 additions & 0 deletions test/jobs/verify_link_job_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit ea62255

Please sign in to comment.