If you upload an html document as a file attachment, say myDoc.htm or myDoc.html, ‘.txt’ gets added as a suffix, so its name becomes myDoc.html.txt or myDoc.html.txt.
At one level, this isn’t quite what you want because you may want to link to that document from the body of a post, perhaps opening it in a new window, and have the browser display it, rendering the html. This will not happen with a ‘.txt’ suffix. Also you may want Users to download a working html file, without their having to remove the newly attached ‘.txt’ suffix.
I believe the code that does this adding ‘.txt’ is this
// Rename potentially executable files, to help prevent exploits.
if (((substr($file->filemime, 0, 5) == 'text/' || strpos($file->filemime, 'javascript')) && (substr($file->filename, -4) != '.txt')) || preg_match('/\.(php|pl|py|cgi|asp)$/i', $file->filename)) {
$file->filemime = 'text/plain';
$file->filepath .= '.txt';
$file->filename .= '.txt';
}
in the file includes/file.inc
My question is might we rewrite this code to exempt html files from it ie if the filemime is ‘text/html’ and the suffix is ‘,htm’ or ‘html’, might we just leave them alone?
Or are files text/html and .htm or .html also potentially executable and thus a security risk?
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | renamedangerousuploads.patch_0.txt | 1.61 KB | breyten |
| #2 | renamedangerousuploads.patch.txt | 1.03 KB | breyten |
Comments
Comment #1
mfricke commentedSorry, I think this was posted in the wrong place. It is to do with upload, not the upload(simple) module.
Comment #2
breyten commentedThis was/is bugging me today in developing a new Drupal site. Not HTML files btw, but javascript files. It means that it's currently impossible to properly upload a greasemonkey script, for example.
Because there are legitimate cases where you want to be able to upload these files, even if they are considered to be dangerous, this behavior should be overridable.
Patch attached against current CVS, untested. (But it's a simple change)
Comment #3
beginner commentedWhere is file_rename_dangerous_uploads defined? I grepped for it but cannot find it.
Also, if you want to create a setting to override the .txt being appended, then you must create this setting per role: the admins and moderators may for example be trusted to upload files with potentially dangerous file extensions, but in a community web site, it is impossible to trust everybody.
Comment #4
breyten commentedSettings per role is a lot smarter indeed. Thanks for the suggestion. New patch attached.
Comment #5
beginner commentedAccording to http://drupal.org/node/78807 , the offending code should be removed altogether, in which case this patch is not needed after all...
Can you check with Darren Oh ?
Comment #6
darren ohThe .txt extension was supposed to be added to files whose type couldn't be identified. However, PHP mime type checking was unreliable, so the code to check types was removed from Drupal. The result was that all files have the .txt extension added.
This patch may be a good interim solution while we wait for someone to contribute reliable code for checking mime types (maybe after the next version of PHP is released). On the other hand, Drupal already allows administrators to specify the file extensions which can be uploaded. That may be the best security we can offer at this point.
Comment #7
darren ohThe features provided by this patch seem to either duplicate existing functionality or become unnecessary when the patch in issue 78807 is applied. Since there have been no more posts to this issue, I'm marking it as "won't fix".