From: Gustaf N. <ne...@wu...> - 2013-07-04 09:39:20
|
Hi Jeff, many thanks for the feedback. The logic is as follows: - when gzip_static is activated, then fastpath checks for the existence of a .gz file and uses it if not outdated. - if gzip_static is activated and there exists a gzip file and it is outdated, then the gzipcmd comes into play to update it, but only, when it is configured (by default it is not configured). So an attack vector is more complex than you sketched, but i guess possible, when gzipcmd was activated. In general, you are right: using a tcl proc is better since it makes it easy to change the behavior without touching C. I have added a boolean parameter "gzip_refresh" to make the intention of the site-admin for refreshing gzip files on the fly clear. A new tcl command "ns_gzipfile" is used now for gzipping (this command requires tcl 8.5 to work) https://bitbucket.org/naviserver/naviserver/commits/f7dca733625553ab802c93d35d471524e5a13e3e all the best -gustaf neumann Am 04.07.13 04:38, schrieb Jeff Rogers: > Gustaf Neumann wrote: > >> When the gzip_cmd is configured, NaviServer keeps track of >> updating the .gz file for the cases the source file is updated. >> There is no burden for the admin. The gzip command is >> called via Tcl "exec", which can in turn be executed via >> nsproxy (see e.g. OpenACS). I took this approach >> over an in-memory variant to avoid memory bloats in >> case huge gzip files should be compressed. Note that >> the compression overhead is typically just a one-time >> operation. The website maintained defines, what >> files should be sent gzipped by compressing these >> once. > I think there's a potential security hole here. I didn't come up with a > proper exploit, but if a user can get control of a filename (e.g., if > there is an ability to upload files), then an arbitrary string could get > passed to the exec command, including but not limited to [] (which would > let tcl do commmand expansion) or spaces (which could cause the filename > to be interpreted as multiple words and hijack the exec behavior). > > Using Tcl_DStringAppendElement instead of Tcl_DStringAppend should > prevent this, as it will force the filename to be a proper list element. > > Alternately, it would be more flexible to change the definition of the > zipCmd to be a tcl command that is passed the filename and zipfile name, > e.g., "gzip_cmd file file.gz", with the tcl definition of gzip_cmd > choosing how to handle it, whether by exec or compressing in-process > (e.g., with 'zlib compress'), or choosing based on the file size. > > -J > |