Bug 6696

Summary: Uploads must not use supplied file extension
Product: OJS Reporter: Alec Smecher <alec>
Component: EditorsAssignee: PKP Support <pkp-support>
Status: RESOLVED FIXED    
Severity: normal CC: ales.kladnik, colin.prince
Priority: P3    
Version: 2.3.6   
Hardware: All   
OS: All   
Version Reported In: 2.3.5 Also Affects:
Attachments: Patch against OJS 2.3.3, 2.3.4, 2.3.5
Patch against OJS 2.2.1, 2.2.2, 2.2.3, 2.2.4, 2.3.0, 2.3.1, 2.3.2

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