On Wed, 4 Jan 2006, Sebastian Kanthak wrote:
i'm trying to debug something (file_column plugin) which makes use of
RAILS_ROOT to determine a root storage path
root_path = File::join RAILS_ROOT, "public"
that's well enough - but this same path is used throughout the code to
generate urls for files under root_path. my understanding of RAILS_ROOT
and the "public" subdir is that one should never be generating links from
outside of "public" in this way since it subverts security at minimum and,
at maximum, is broken since a url relative to RAILS_ROOT is not guaranteed
to be visible since RAILS_ROOT is a file_system concept and is not in url
space. is this correct?
Yes, that's correct as far as I understand it. I suppose this is in
conjunction with the problem you ran in to with file_column and Family
Connection? If so, I wonder if Sebastian has any comment on implementation
choices that he had to make.
well, my goal for file_column was to reduce the setup required to a
minimum. My basic assumption are that
* RAILS_ROOT/public is a directory I can write files to
* files in RAILS_ROOT/public are accessible via the webapps base URL
(i.e., the URL that is used for url_for, etc.)
These assumptions are true for webrick and most setups I've seen so
far. It can be changed by using the following configuration options
for file_column:
* :root_path, this defaults to RAILS_ROOT
* :web_root, the URL where to reach :root_path, relative to the
rails app's base URL, defaults to ""
Ara, what security issues does this setup raise? Have you any
suggestions on how to improve it?
here's the issues and how i fixed them - the problem involves both the
file_column plugin and the familyconnection software, but could easily affect
other rails projects:
assume the following
- rails project living in
/var/www/ror/howards
- the public directory is exposed thus
/var/www/html/howards -> /var/www/ror/howards/public
where the first is a link
================================================================
problem one (filecolumn plugin):
================================================================
file: file_column/lib/file_column.rb
172 def absolute_path(subdir=nil)
173 if subdir
174 # File.expand_path(File.join(@dir, subdir, @filename)) # fix
175 File.join(@dir, subdir, @filename)
176 else
177 # File.expand_path(File.join(@dir, @filename)) # fix
178 File.join(@dir, @filename)
179 end
180 end
the problem and fix are shown. RAILS_ROOT will have a value of something like
/var/www/html/howards/../config/../
but recall that
/var/www/html/howards/
is a link. when it is expanded it becomes something like
/var/www/html/
which is obviously not correct. this is because:
[EMAIL PROTECTED] ahoward]# irb
irb(main):001:0> path =
"/var/www/html/howards/../config/../public/photo/image/1/FishRobert.jpg"
=> "/var/www/html/howards/../config/../public/photo/image/1/FishRobert.jpg"
irb(main):002:0> File::exist? path
=> true
irb(main):003:0> expanded_path = File::expand_path path
=> "/var/www/html/public/photo/image/1/FishRobert.jpg"
irb(main):004:0> File::exist? expanded_path
=> false
so the call to File::expand_path destroys the validity of the path if any
links are encountered. this can be fixed using
Pathname::new(path).realpath.to_s
which deals with links correctly, but only on *nix. the suggested fix is
simply not to expand the paths - it's not needed for a method called
'absolute_path' anyhow as the returned path is certainly absolute, if not
expanded, with the fix.
================================================================
problem two (filecolumn plugin):
================================================================
file: file_column/lib/file_column_helper.rb
45 def url_for_file_column(object, method, subdir=nil)
46 case object
47 when String, Symbol
48 object = instance_variable_get("@#{object.to_s}")
49 end
50 relative_path = object.send("#{method}_relative_path", subdir)
51 return nil unless relative_path
52 url = ""
53 # url << @request.relative_url_root.to_s << "/" # fix
54 url << "/"
55 url << object.send("#{method}_options")[:base_url] << "/"
56 url << relative_path
57 end
the issue here is that adding the 'relative_url_root' causes it to be added
twice since the rails trio of 'image_tag/image_path/compute_public_path'
already do this work. in short the urls returned should be relative to the
application so the other rails helpers deal with them correctly. note that
one would not see this behaviour of 'relative_url_root' was empty!
oncee that is fixed it causes...
================================================================
problem three (familyconnection project)
================================================================
file: app/views/news/_news_item.rhtml (and perhaps others!!)
6 <% if news_item.photos_count > 0 -%>
7 <% for photo in news_item.photos -%>
8 <%
9 thumb_url = url_for_image_column(photo, "image",
10 :size => "50x50", :crop => "1:1", :name => "thumb")
11 # fullsize_url = url_for_image_column(photo, "image", :name =>
"fullsize") # fix
12 fullsize_url = image_path(url_for_image_column(photo, "image", :name =>
"fullsize"))
13 %>
14 <%= link_to_function(image_tag(thumb_url),
"popitup('#{fullsize_url}', 542, 532)") %>
15 <% end -%>
16 <div style="clear: both"></div>
17 <% end -%>
once 'url_for_file_column' is changed to return urls which are relative to
the application we have to be careful to use all the rails' helper methods
to build correct urls. in this case without the fix
link_to_function(image_tag(thumb_url), "popitup('#{fullsize_url}', 542,
532)")
will correctly show the 'thumb_url' but the 'popitup' method will fail since
'fullsize_url' will not have 'relative_url_root' added.
i don't know a way around this since we need to pass urls from
'url_for_file_column' to 'image_tag', which will prepend the
'relative_url_root' to them, but we'd assume a method named
'url_for_file_column' would return a valid url - however this is not the
case __unless__ our app is sitting in the webroot and 'relative_url_root' is
empty.
it strikes me as odd that 'compute_public_path' would add the
'relative_url_root' to a path which has already had it added, but looking at
file: actionpack-1.11.2/lib/action_view/helpers/asset_tag_helper.rb
148 def compute_public_path(source, dir, ext)
149 source = "/#{dir}/#{source}" unless source.first == "/" ||
source.include?(":")
150 source = "#{source}.#{ext}" unless source.include?(".")
151 source = "[EMAIL PROTECTED]" unless %r{^[-a-z]+://} =~ source
152 source = ActionController::Base.asset_host + source unless
source.include?(":")
153 source
154 end
we can see on line 151 that it certainly is __always__ added unless the url
has a protocol attached (which seems an odd test doesn't it?).
another approach might be to make 'url_for_file_column' return absolute urls
of the form
http://server:port/path/to/image.jpg
and trust that rails helper methods will leave it alone since it's certainly
an absolute url... but that didn't smell right to me.
in any case, both peice of software certainly are great and i'll be using
both. a test run of the familyconnection is up at
http://fortytwo.merseine.nu/howards/
and you can see all the fixes working here
http://fortytwo.merseine.nu/howards/news
in case either of you are doubting thomases like i am.
thanks for the great software - hopefully this info will help you.
cheers.
-a
--
===============================================================================
| ara [dot] t [dot] howard [at] noaa [dot] gov
| all happiness comes from the desire for other be happy. all misery
| comes from the desire for oneself to be happy.
| -- bodhicaryavatara
===============================================================================
_______________________________________________
Rails-core mailing list
Rails-core@lists.rubyonrails.org
http://lists.rubyonrails.org/mailman/listinfo/rails-core
