Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Guard against SSRF attacks in link verification #5076

Merged
merged 1 commit into from
Sep 30, 2024
Merged

Guard against SSRF attacks in link verification #5076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Gemfile
View file
Open in desktop
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", require: "faraday/restrict_ip_addresses"
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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