Closed (fixed)
Project:
Drupal core
Component:
base system
Priority:
Critical
Category:
Feature request
Assigned:
Reporter:
Created:
21 Aug 2005 at 23:26 UTC
Updated:
16 Dec 2005 at 10:20 UTC
Jump to comment: Most recent file
Here's patch/proposal to add the .tpl file extension (used for smarty theme/module templates) to the list of those denied in the base .htaccess file.
--- .htaccess 2005-08-22 18:22:14.799176000 +0000
+++ .htaccess_old 2005-08-22 18:22:48.287329600 +0000
@@ -3,7 +3,7 @@
#
# Protect files and directories from prying eyes.
-<Files ~ "(\.(inc|module|pl|sh|sql|theme|engine|xtmpl|tpl)|Entries|Repositories|Root|scripts|updates)$">
+<Files ~ "(\.(inc|module|pl|sh|sql|theme|engine|xtmpl)|Entries|Repositories|Root|scripts|updates)$">
Order deny,allow
Deny from all
</Files>
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | _29344_htaccess_0.patch | 646 bytes | morbus iff |
| #4 | _29344_htaccess.patch | 648 bytes | morbus iff |
| #1 | tpl_file_hiding_0.patch | 439 bytes | tclineks |
| tpl_file_hiding.patch | 439 bytes | tclineks |
Comments
Comment #1
tclineks commentedAhem...
Comment #2
killes@www.drop.org commentedIt's a patch.
Comment #3
Uwe Hermann commentedSee also http://drupal.org/node/28776 which adds more stuff to .htaccess.
Comment #4
morbus iffI've attached an patch that updates the .htaccess "protect files" block to real world sanity.
* Uses [FilesMatch] not [Files ~], per Apache.
* Removed "updates" from the list; presumably this was database/updates.inc, which is covered with .inc.
* Removed "scripts", a directory; we're only matching Files (all its files are covered by extensions anyways).
* Turned "Entries" into "Entries.*", to cover a possible CVS/Entries.log file.
* Added tpl, per this Issue's original purpose.
Uwe comments in another issue (linked above) that databases/ and the files within should be blocked. Welp, updates.inc is already being blocked by the .inc, and there's absolutely no reason to block the database files: they contain no sensitive information, nor are they executable. Likewise, they're easily found elsewhere (like, in our CVS). I have NOT, however, removed the .sql extension: there IS a chance that someone COULD dump a .sql file that DOES contain sensitive information, and that should be preempted.
I've also changed the .pl filter to "code-style\.pl", which requires a little more explanation. .pl is a Perl script and, in our case, we're apparently attempting to protect scripts/code-style.pl. But, there's a very legitimate use for .pl scripts in normal web operation, even more so when an Apache administrator has allowed .cgi and .pl scripts to be executed anywhere (with ExecCGI and AddHandler, a common tactic explained in Apache's FAQ). Now, in some cases, you may say "well, this would protect against people uploading a .pl script via Drupal and running it, right?!", but then we'd also have to worry about stuff like .cgi and .py and .php and so forth. Drupal actually does that elsewhere: in its file_check_upload(). The more specific change to "code-style\.pl" continues to protect our only shipped .pl file, without losing any true loss of security (if blocking .pl scripts WAS meant as "security", then it was pretty lackluster, as evidenced by the much stronger, and Drupal-specific, checks in file_check_upload()).
I'm also setting this patch to critical: a .tpl file COULD contain sensitive information.
Comment #5
morbus iffPatch updated: changed "Repositories" to "Repository". According to the CVS book, there is no "Repositories".
Comment #6
chx commentedYou sure we can rely on everyone upgraded to Apache 1.3 by now? It was released quite recently, 6th June 1998 :)
Comment #7
morbus iffHeh, heh. Yeah, I'd say so ;)
Comment #8
tclineks commentedJust putting my +1 in here.
/me sulks because of content of initial issue post
I'm working on updates to the Smarty Theme Engine to prepare for 4.7 and would like to roll out many theme conversions alongside the tagging of the branch. Preferably they won't each need a separate .htaccess file to prevent tpl file viewing.
Though out of the box they contain relatively benign information but I'm sure they are used in 'creative' ways down the line and should be concealed.
Oh, Just updated my HEAD wc and noticed dries has committed it, marking as fixed.
Comment #9
killes@www.drop.org commentedThis still needs committing.
Comment #10
killes@www.drop.org commentedoops, got committed.
Comment #11
(not verified) commented