From 051c181ef436bcd85871226cc01382e08b495954 Mon Sep 17 00:00:00 2001 From: motoyasu-saburi Date: Sun, 27 Nov 2022 14:04:19 +0900 Subject: [PATCH] escape filename in the multipart/form-data Content-Disposition header --- lib/httparty/request/body.rb | 9 ++++++++- spec/httparty/request/body_spec.rb | 32 ++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/lib/httparty/request/body.rb b/lib/httparty/request/body.rb index 4479e1b6..12884425 100644 --- a/lib/httparty/request/body.rb +++ b/lib/httparty/request/body.rb @@ -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) } @@ -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 diff --git a/spec/httparty/request/body_spec.rb b/spec/httparty/request/body_spec.rb index b6c7bb7b..3bfc574d 100644 --- a/spec/httparty/request/body_spec.rb +++ b/spec/httparty/request/body_spec.rb @@ -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