Skip to content

Commit 17507e8

Browse files
travisptenderlove
authored andcommitted
Include Content-Length in signature for ActiveStorage direct upload
[CVE-2020-8162]
1 parent b738f19 commit 17507e8

File tree

2 files changed

+25
-1
lines changed

2 files changed

+25
-1
lines changed

activestorage/lib/active_storage/service/s3_service.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,8 @@ def url(key, expires_in:, filename:, disposition:, content_type:)
8181
def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:)
8282
instrument :url, key: key do |payload|
8383
generated_url = object_for(key).presigned_url :put, expires_in: expires_in.to_i,
84-
content_type: content_type, content_length: content_length, content_md5: checksum
84+
content_type: content_type, content_length: content_length, content_md5: checksum,
85+
whitelist_headers: ['content-length']
8586

8687
payload[:url] = generated_url
8788

activestorage/test/service/s3_service_test.rb

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,29 @@ class ActiveStorage::Service::S3ServiceTest < ActiveSupport::TestCase
3030
@service.delete key
3131
end
3232

33+
test "directly uploading file larger than the provided content-length does not work" do
34+
key = SecureRandom.base58(24)
35+
data = "Some text that is longer than the specified content length"
36+
checksum = Digest::MD5.base64digest(data)
37+
url = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size - 1, checksum: checksum)
38+
39+
uri = URI.parse url
40+
request = Net::HTTP::Put.new uri.request_uri
41+
request.body = data
42+
request.add_field "Content-Type", "text/plain"
43+
request.add_field "Content-MD5", checksum
44+
upload_result = Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http|
45+
http.request request
46+
end
47+
48+
assert_equal "403", upload_result.code
49+
assert_raises ActiveStorage::FileNotFoundError do
50+
@service.download(key)
51+
end
52+
ensure
53+
@service.delete key
54+
end
55+
3356
test "upload a zero byte file" do
3457
blob = directly_upload_file_blob filename: "empty_file.txt", content_type: nil
3558
user = User.create! name: "DHH", avatar: blob

0 commit comments

Comments
 (0)