Skip to content

Commit

Permalink
Merge pull request from GHSA-5pq7-52mg-hr42
Browse files Browse the repository at this point in the history
escape filename in the multipart/form-data Content-Disposition header
  • Loading branch information
jnunemaker authored Dec 30, 2022
2 parents 243a215 + 051c181 commit cdb45a6
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 1 deletion.
9 changes: 8 additions & 1 deletion lib/httparty/request/body.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ def multipart?

private

# https://html.spec.whatwg.org/#multipart-form-data
MULTIPART_FORM_DATA_REPLACEMENT_TABLE = {
'"' => '%22',
"\r" => '%0D',
"\n" => '%0A'
}.freeze

def generate_multipart
normalized_params = params.flat_map { |key, value| HashConversions.normalize_keys(key, value) }

Expand All @@ -40,7 +47,7 @@ def generate_multipart
memo << %(Content-Disposition: form-data; name="#{key}")
# value.path is used to support ActionDispatch::Http::UploadedFile
# https://github.com/jnunemaker/httparty/pull/585
memo << %(; filename="#{file_name(value)}") if file?(value)
memo << %(; filename="#{file_name(value).gsub(/["\r\n]/, MULTIPART_FORM_DATA_REPLACEMENT_TABLE)}") if file?(value)
memo << NEWLINE
memo << "Content-Type: #{content_type(value)}#{NEWLINE}" if file?(value)
memo << NEWLINE
Expand Down
32 changes: 32 additions & 0 deletions spec/httparty/request/body_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,38 @@

it { is_expected.to eq multipart_params }
end

context 'when file name contains [ " \r \n ]' do
let(:options) { { force_multipart: true } }
let(:some_temp_file) { Tempfile.new(['basefile', '.txt']) }
let(:file_content) { 'test' }
let(:raw_filename) { "dummy=tampering.sh\"; \r\ndummy=a.txt" }
let(:expected_file_name) { 'dummy=tampering.sh%22; %0D%0Adummy=a.txt' }
let(:file) { double(:mocked_action_dispatch, path: some_temp_file.path, original_filename: raw_filename, read: file_content) }
let(:params) do
{
user: {
attachment_file: file,
enabled: true
}
}
end
let(:multipart_params) do
"--------------------------c772861a5109d5ef\r\n" \
"Content-Disposition: form-data; name=\"user[attachment_file]\"; filename=\"#{expected_file_name}\"\r\n" \
"Content-Type: text/plain\r\n" \
"\r\n" \
"test\r\n" \
"--------------------------c772861a5109d5ef\r\n" \
"Content-Disposition: form-data; name=\"user[enabled]\"\r\n" \
"\r\n" \
"true\r\n" \
"--------------------------c772861a5109d5ef--\r\n"
end

it { is_expected.to eq multipart_params }

end
end
end
end
Expand Down

0 comments on commit cdb45a6

Please sign in to comment.