Bug 6696 - Uploads must not use supplied file extension
Uploads must not use supplied file extension
Status: RESOLVED FIXED
Product: OJS
Classification: Unclassified
Component: Editors
2.3.6
All All
: P3 normal
Assigned To: PKP Support
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-20 11:19 PDT by Alec Smecher
Modified: 2014-10-07 12:38 PDT (History)
2 users (show)

See Also:
Version Reported In: 2.3.5
Also Affects:


Attachments
Patch against OJS 2.3.3, 2.3.4, 2.3.5 (2.13 KB, patch)
2011-06-20 11:30 PDT, Alec Smecher
Details | Diff
Patch against OJS 2.2.1, 2.2.2, 2.2.3, 2.2.4, 2.3.0, 2.3.1, 2.3.2 (2.25 KB, patch)
2011-06-20 11:36 PDT, Alec Smecher
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alec Smecher 2011-06-20 11:19:38 PDT
File uploads into public should use a file extension validated via whitelist by the file manager (e.g. PublicFileManager::getImageExtension) rather than the file extension supplied by the client (e.g. PublicFileManager::getExtension) or detected by the server (e.g. mime_content_type).
Comment 1 Alec Smecher 2011-06-20 11:30:10 PDT
Created attachment 3568 [details]
Patch against OJS 2.3.3, 2.3.4, 2.3.5
Comment 2 Alec Smecher 2011-06-20 11:36:44 PDT
Created attachment 3569 [details]
Patch against OJS 2.2.1, 2.2.2, 2.2.3, 2.2.4, 2.3.0, 2.3.1, 2.3.2
Comment 3 Alec Smecher 2011-06-20 11:40:46 PDT
NOTE: The patch for bug #6689 must be applied before these patches will apply cleanly.
Comment 4 Ales Kladnik 2011-06-21 03:33:46 PDT
(In reply to comment #0)
> File uploads into public should use a file extension validated via whitelist by
> the file manager (e.g. PublicFileManager::getImageExtension) rather than the
> file extension supplied by the client (e.g. PublicFileManager::getExtension) or
> detected by the server (e.g. mime_content_type).

I applied this patch after #6689, but the behaviour is now a bit strange. When uploading a cover image file for an issue, I could still upload a file named "setup.exe", however now it gets renamed to "cover_issue_120_en_US.flv". I tried also extensions php, js, tlp, pdf, and they were not accepted. Then I tried several other .exe files and all were accepted and renamed to .flv after upload.
Comment 5 Alec Smecher 2011-06-21 08:41:42 PDT
Ales, the file type check works by getting the file's MIME type, then mapping that to a file extension according to this list:

image/gif: .gif
image/jpeg, image/pjpeg: .jpg
image/png, image/x-png: .png
image/vnd.microsoft.icon, image/x-icon, image/ico: .ico
application/x-shockwave-flash: .swf
video/x-flv, application/x-flash-video, flv-application/octet-stream, application/octet-stream: .flv
audio/mpeg: .mp3
audio/x-aiff: .aiff
audio/x-wav: .wav
video/mpeg: .mpg
video/quicktime: .mov
video/mp4: .mp4
text/javascript: .js

Anything not in this list is not allowed. It's used for many parts of the system to allow multimedia content (e.g. images in articles).

What you're probably seeing is an .exe detected as application/octet-stream, which maps to .flv for flash video. (This entry is for compatibility with systems that don't have a specific MIME entry for .flv files.) As long as the file extension is NOT trusted i.e. as .exe or .php, the web server shouldn't invoke the executable.

I suppose as a tune-up we could limit certain areas to only plain old image files, i.e. PDF, PNG, and GIF, as per the error messages...
Comment 6 Alec Smecher 2014-10-07 12:38:26 PDT
Removed application/octet-stream
https://github.com/pkp/pkp-lib/commit/602532ea5c1093fc63b98222dc457709dfee0119