From c0ab9a7d29b84a6ccb1ffa3e8ca1ce61f7a9fbb8 Mon Sep 17 00:00:00 2001
From: Travis Pew <travisp@gmail.com>
Date: Thu, 14 May 2020 11:17:46 -0400
Subject: [PATCH] Include Content-Length in signature for ActiveStorage direct
 upload

[CVE-2020-8162]
---
 .../lib/active_storage/service/s3_service.rb  |  3 ++-
 activestorage/test/service/s3_service_test.rb | 23 +++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/activestorage/lib/active_storage/service/s3_service.rb b/activestorage/lib/active_storage/service/s3_service.rb
index fa2ab911cda10..b4fba3d6ec1ff 100644
--- a/activestorage/lib/active_storage/service/s3_service.rb
+++ b/activestorage/lib/active_storage/service/s3_service.rb
@@ -80,7 +80,8 @@ def exist?(key)
     def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:)
       instrument :url, key: key do |payload|
         generated_url = object_for(key).presigned_url :put, expires_in: expires_in.to_i,
-          content_type: content_type, content_length: content_length, content_md5: checksum, **upload_options
+          content_type: content_type, content_length: content_length, content_md5: checksum
+          whitelist_headers: ['content-length'], **upload_options
 
         payload[:url] = generated_url
 
diff --git a/activestorage/test/service/s3_service_test.rb b/activestorage/test/service/s3_service_test.rb
index a65fe42720364..5ebfd16cc33bb 100644
--- a/activestorage/test/service/s3_service_test.rb
+++ b/activestorage/test/service/s3_service_test.rb
@@ -55,6 +55,29 @@ class ActiveStorage::Service::S3ServiceTest < ActiveSupport::TestCase
       @service.delete key
     end
 
+    test "directly uploading file larger than the provided content-length does not work" do
+      key      = SecureRandom.base58(24)
+      data     = "Some text that is longer than the specified content length"
+      checksum = Digest::MD5.base64digest(data)
+      url      = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size - 1, checksum: checksum)
+
+      uri = URI.parse url
+      request = Net::HTTP::Put.new uri.request_uri
+      request.body = data
+      request.add_field "Content-Type", "text/plain"
+      request.add_field "Content-MD5", checksum
+      upload_result = Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http|
+        http.request request
+      end
+
+      assert_equal "403", upload_result.code
+      assert_raises ActiveStorage::FileNotFoundError do
+        @service.download(key)
+      end
+    ensure
+      @service.delete key
+    end
+
     test "upload a zero byte file" do
       blob = directly_upload_file_blob filename: "empty_file.txt", content_type: nil
       user = User.create! name: "DHH", avatar: blob