Merge pull request #122 from basecamp/unfurl-blind-ssrf

Security: disallow blind SSRF to link-local IPs via URL unfurling

lib/rails_ext/action_text_attachables.rb

@@ -16,13 +16,18 @@ ActiveSupport.on_load(:action_text_content) do
 
             decoded_gid = if data = encoded_message.dig("_rails", "data")
               data
+            elsif data = encoded_message.dig("_rails", "message")
+              # Rails 7 used an older format of GID that serialized the payload using Marshall
+              # Since we intentionally skip signature verification, we can't safely unmarshal the data
+              # To work around this, we manually extract the GID from the marshaled data
+              Base64.urlsafe_decode64(data).match(%r{(gid://campfire/[^/]+/\d+)})&.to_s
             else
               nil
             end
 
-            model = GlobalID.find(decoded_gid)
+            if model = GlobalID.find(decoded_gid)
-
+              model.model_name.to_s.in?(ATTACHABLES_PERMITTED_WITH_INVALID_SIGNATURES) ? model : nil
-            model.model_name.to_s.in?(ATTACHABLES_PERMITTED_WITH_INVALID_SIGNATURES) ? model : nil
+            end
           end
         rescue ActiveRecord::RecordNotFound
           nil

lib/restricted_http/private_network_guard.rb

@@ -16,7 +16,7 @@ module RestrictedHTTP
 
     def private_ip?(ip)
       IPAddr.new(ip).then do |ipaddr|
-        ipaddr.private? || ipaddr.loopback? || LOCAL_IP.include?(ipaddr)
+        ipaddr.private? || ipaddr.loopback? || ipaddr.link_local? || ipaddr.ipv4_mapped? || LOCAL_IP.include?(ipaddr)
       end
     rescue IPAddr::InvalidAddressError
       true

test/lib/rails_ext/action_text_attachables_test.rb

@@ -0,0 +1,39 @@
+require "test_helper"
+
+class ActionText::AttachmentTest < ActiveSupport::TestCase
+  setup do
+    @user = users(:david)
+  end
+
+  test "from_node" do
+    html = %Q(<action-text-attachment sgid="#{@user.attachable_sgid}"></action-text-attachment>)
+    node = ActionText::Fragment.wrap(html).find_all(ActionText::Attachment.tag_name).first
+
+    attachment = ActionText::Attachment.from_node(node)
+    assert_equal @user, attachment.attachable
+  end
+
+  test "from_node with a Rails 7 SGID" do
+    gid = @user.to_gid.to_s
+    marshaled_gid = Base64.urlsafe_encode64(Marshal.dump(gid))
+    rails7_payload = { "_rails" => { "message" => marshaled_gid, "exp" => nil, "pur" => "attachable" } }
+    rails7_message = Base64.strict_encode64(JSON.generate(rails7_payload))
+    rails7_sgid = "#{rails7_message}--invalidsignature"
+
+    html = %Q(<action-text-attachment sgid="#{rails7_sgid}"></action-text-attachment>)
+    node = ActionText::Fragment.wrap(html).find_all(ActionText::Attachment.tag_name).first
+
+    attachment = ActionText::Attachment.from_node(node)
+    assert_equal @user, attachment.attachable
+  end
+
+  test "from_node with an invalid SGID" do
+    room = rooms(:pets).tap { |r| r.extend ActionText::Attachable }
+
+    html = %Q(<action-text-attachment sgid="#{room.attachable_sgid}invalid"></action-text-attachment>)
+    node = ActionText::Fragment.wrap(html).find_all(ActionText::Attachment.tag_name).first
+
+    attachment = ActionText::Attachment.from_node(node)
+    assert_kind_of ActionText::Attachables::MissingAttachable, attachment.attachable
+  end
+end

test/models/opengraph/location_test.rb

@@ -21,6 +21,28 @@ class Opengraph::LocationTest < ActiveSupport::TestCase
     assert_equal [ "is not public" ], location.errors[:url]
   end
 
+  test "link-local addresses are blocked" do
+    Resolv.stubs(:getaddress).with("metadata.internal").returns("169.254.169.254")
+
+    location = Opengraph::Location.new("https://metadata.internal")
+    assert_not location.valid?
+    assert_equal [ "is not public" ], location.errors[:url]
+  end
+
+  test "IPv6 addresses mapped to IPv4 addresses are blocked" do
+    Resolv.stubs(:getaddress).with("metadata.internal").returns("::ffff:192.168.1.1")
+
+    location = Opengraph::Location.new("https://metadata.internal")
+    assert_not location.valid?
+    assert_equal [ "is not public" ], location.errors[:url]
+
+    Resolv.stubs(:getaddress).with("metadata.internal").returns("::ffff:c0a8:0101")
+
+    location = Opengraph::Location.new("https://metadata.internal")
+    assert_not location.valid?
+    assert_equal [ "is not public" ], location.errors[:url]
+  end
+
   test "avoid reading file urls when expecting HTML" do
     large_file = Opengraph::Location.new("https://www.example.com/100gb.zip")