6

I'm using send_file on a Sinatra app:

get '/update/dl/:upd' do

    filename ="/uploads/#{params[:upd]}"
    send_file(filename, :filename => "t.cer", :type => "application/octet-stream")
end

The folder /uploads/ it's not public, it's on the app dir. When I try to go to localhost:4567/update/dl/some_file in Chrome it returns me a 404, like with Firefox, when seeing the headers, it's a 404. But if I try with Safari it downloads the file. So I guess somthing's wrong with my code (and Safari's, but let's left that to Apple :P). What could be wrong? Thanks!

pmerino
  • 5,900
  • 11
  • 57
  • 76

1 Answers1

3

I get it to work fine in chrome if I remove the initial slash in filename so it's "filename instead of "/filename. The 404 comes from a file not found error in send_file

# foo.rb
require 'sinatra'
get '/update/dl/:upd' do
    filename ="uploads/#{params[:upd]}"
    # just send the file if it's an accepted file
    if filename =~ /^[a-zA-Z0-9]*.cer$/
      send_file(filename, :filename => "t.cer", :type => "application/octet-stream")
    end
end

However, there's really a big security hole in this, a user can download anything that the sinatra process has access too, I named my sinatra app foo.rb and this request downloads the sinatra script:

 http://localhost:4567/update/dl/..%2Ffoo.rb
sunkencity
  • 3,482
  • 1
  • 22
  • 19
  • No big deal, only trusted company members will have access – pmerino Dec 28 '11 at 20:07
  • OK, I added a simple check, it's an unnecessary attack vector to leave open in any case. – sunkencity Dec 28 '11 at 20:16
  • Sinatra by default uses rack-protection with path traversal protection enabled, so this check is unnecessary. (EDIT: this may not have been true at the time the answer was written but its certainly true now) – nightpool Jul 07 '15 at 22:14