Sanitize user-generated filenames and only send files inside a given directory

Updated . Posted . Visible to the public.

If in your application your users pass along params that result in filenames, like invoices/generated?number=123. This could be your (very careless) controller method:

def generated
  send_file File.join(Rails.root, 'shared', 'invoices', params[:number])
end

This allows your users not only to access those files but also any files your application can read, like this:

invoices/generated?number=../../../../../etc/passwd
# => send_file '/etc/passwd'

You do not want this. In most cases you should prefer a show method that does not open files directly from the file system -- the following is for those rare cases in which you can not do that.


First, check if the requested file is actually at the place you would want it to be and only send it if that is the case.

You can use the following trait Show archive.org snapshot and always call send_file_inside with the parent directory of the files as the first parameter, like so:

send_file_inside File.join(Rails.root, 'shared', 'invoices'), params[:number]

Do not use only Rails.root -- this would allow access to config/environment.rb and other sensitive data inside your application's directory.

Put this into app/controllers/shared/send_file_inside_trait.rb:

module ApplicationController::SendFileInsideTrait
  as_trait do

    private

    def send_file_inside(allowed_path, filename, options = {})
      path = File.expand_path(File.join(allowed_path, filename))
      if path.match Regexp.new('^' + Regexp.escape(allowed_path))
        send_file path, options
      else
        raise 'Disallowed file requested'
      end
    end

  end
end

Here is the spec that goes along with it:

require 'spec_helper'

describe ApplicationController do

  describe '#send_file_inside' do

    let(:path) { '/opt/invoices/' }

    it 'should send a requested file that lives beneath the given directory' do
      subject.should_receive(:send_file).with('/opt/invoices/123.pdf', {})
      subject.send :send_file_inside, path, '123.pdf'
    end

    it 'should raise an error if a file outside the given directory was requested' do
      subject.should_not_receive :send_file
      lambda{ subject.send :send_file_inside, path, '../../etc/passwd' }.should raise_error('Disallowed file requested')
    end

    it 'should work with subdirectories' do
      subject.should_receive(:send_file).with('/opt/invoices/2011/001.pdf', {})
      subject.send :send_file_inside, path, '2011/001.pdf'
    end

    it 'should pass along given options to the send_file method' do
      subject.should_receive(:send_file).with('/opt/invoices/123.pdf', :given_options)
      subject.send :send_file_inside, path, '123.pdf', :given_options
    end

  end

end
Thomas Eisenbarth
Last edit
Henning Koch
License
Source code in this card is licensed under the MIT License.
Posted by Thomas Eisenbarth to makandra dev (2011-03-21 11:34)