This project is archived and is in readonly mode.

#5826 ✓hold
Claudio Poli

Incorrect transformation of file name upload

Reported by Claudio Poli | October 17th, 2010 @ 02:28 AM | in 2.x

In Rails 2-3-stable I have this with Rack 1.1.0:

If we upload a file with double quotes in file name, like: 01 "Our" Dawn Of Glory.mp3,
we can observe the following in this file:
http://github.com/rails/rails/blob/2-3-stable/actionpack/lib/action...

value will contain:

{:head=>"Content-Disposition: form-data; name=\"media\"; filename=\"01 \"Our\" Dawn Of Glory.mp3\"\r\nContent-Type: application/octet-stream\r\n", :type=>"application/octet-stream", :tempfile=>#<File:/var/folders/M4/M4NWfTkfE7uqHvtn2VpEdk+++TI/-Tmp-/RackMultipart20101017-53020-1xgqkng-0>, :name=>"media", :filename=>"01 "}

Note the :filename parameter gets truncated.
So in original_path we will have an invalid filename:

upload.original_path = value[:filename] # "01 "

This caused issues with plugins such paperclip.

Bad value is being returned from here, which is an override of Rack's GET method:
http://github.com/rails/rails/blob/2-3-stable/actionpack/lib/action...

I cannot test a newer version of Rack on Rails 2-3-stable.

So while the problem may be fixed in recent version of Rack, since Rails 2-3-stable depends on

gem 'rack', '~> 1.1.0'

I think it's worth taking a look, even because plugins such Paperclip tries to delete a file if it encounters error during processes, and it's trying to delete "/tmp/mydir/.". due to filename transformation to determine the content_type.

Also if one have custom cleaning up logic for Files and Directories this can be a potential security issue.

Comments and changes to this ticket

  • Claudio Poli

    Claudio Poli October 17th, 2010 @ 02:48 AM

    • Tag set to rack

    I just tried Rack 1.2.1 on Rails 2-3-stable and I can confirm that this bug is not present.

    It is definitely a Rack problem, now the issue is that 2-3-stable still depends on an old version of Rack.

    A solution would then be ask the Rack developer to release a 1.1.1 from 1.1.0 with this fix backported.
    That way we won't run into issues that Rack 1.2.1 might give but fix this bug.

  • Aditya Sanghi

    Aditya Sanghi October 17th, 2010 @ 05:42 AM

    • State changed from “new” to “hold”
    • Milestone set to 2.x
    • Assigned user set to “José Valim”
    • Importance changed from “” to “Low”

    I'm not sure why we can't move to rack 1.2.1 on 2.3x. But this ticket can be put to hold until there is clarity on the rack version 1.1.x. Perhaps moving to 3.x might be a good idea?

  • Claudio Poli

    Claudio Poli October 17th, 2010 @ 11:58 AM

    There are some outstanding bugs in Rails 3 that prevents me from upgrading right now.

    I opened a ticket to raggi if someone wants to chime in: http://github.com/rack/rack/issues#issue/71

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Tags

Pages