File Upload During Sumbisison

newone · February 1, 2017, 6:23am

Can someone clarify how does having a folder outside of webroot prevent someone from uploading any type of file during submission?

Is there a way to limit the type of file someone can upload?

Can you change the name of the folder that is outside the webroot? Should that cause any issue?

Newone

bozana · February 1, 2017, 1:13pm

Hi @newone

I am not sure I understand the questions correctly, but.

If the folder has no right permissions, e.g. for the web user (e.g. www, www-data) to write that the file cannot be uploaded.
There are probably some things one can check, but the file types can be also manipulated, thus it is not quite secure. But because the OJS files are never directly accessed (if the file folder is not in the web root i.e. OJS directory), but through OJS functions that only provides files for download, they cannot be executed on the server this way.
I think you can do this, but you will have to adjust the setting in config.inc.php as well.

Best,
Bozana

asmecher · February 1, 2017, 4:23pm

Hi @newone,

Arbitrary file type uploads are always allowed by OJS for submission files, and other uploads are limited to a few types (e.g. logos, css, etc).

Moving the upload directory out of the web root is intended to safeguard access to those files against direct requests via the web server, which can 1) expose files to the web without access control, and 2) potentially invoke harmful scripts like .phtml files.

As long as they can’t be accessed directly via the web server, arbitrary file uploads are safely kept on the server, even if they are harmful scripts. Note that restricting uploads to certain types wouldn’t guarantee security – .docx files are technically ZIP archives, so could contain arbitrary files; there have been vulnerabilities in scripts in .pdf files, etc.

Regards,
Alec Smecher
Public Knowledge Project Team

newone · February 1, 2017, 9:27pm

The folder is already outside the web root, which is how it was originally configured, and how OJS recommended it. Our current version is OJS 2.4.8.

I asked because I have noticed that we received some submissions with phtml extensions. When we check submissions via the database, we also deleted the submissions. We have found out that when a user was removed from the website, somehow, whatever submissions they made was still in the database and I am not sure, if the submission folder remain. On comparing the database, we delete the submissions folder that OJS created when the user made that submission. We delete, and do not open.

We generally try to comb through registrations and submissions via the database to check of any suspicious registration and submissions.

I am just wondering if anything else needs to be done.

newone · February 1, 2017, 9:29pm

It would be really nice if there is a way to scan files that users upload during submission. Maybe using ClamAV.

Wishing out loud.

Newone

asmecher · February 1, 2017, 9:34pm

Hi @newone,

How are you removing these users?

I suspect someone is checking to see whether your files_dir is located within your web root by making that submission, and when they find it isn’t, they probably move on.

A virus scan probably wouldn’t find a PHP backdoor, but there are server-side tools that perform automated security scans; have a look e.g. at this article.

Regards,
Alec Smecher
Public Knowledge Project Team

newone · February 1, 2017, 10:43pm

Hi asmecher,

The users are removed using the Merge Users.

There were some accounts that were merged because they are SPAM. I think what happened is that when people submit, and they do not finish their submission, it does not look like their submission folder, which OJS created when they started their submission is deleted when their accounts are merged. I had found a few submissions with the folder in the files_dir, but the users that submitted had already been removed from the website.

Newone

asmecher · February 1, 2017, 10:58pm

Hi @newone,

The “merge users” tool doesn’t delete a user’s submissions, it re-attributes them to the account that they are merged into. So if you merge them into your own account, for example, then those submissions will become yours.

Regards,
Alec Smecher
Public Knowledge Project Team

ctgraham · February 2, 2017, 7:15pm

+1 to this. EPrints provides a similar functionality. I got a little lost looking for the appropriate hook in 3.x, but in 2.4.8, a plugin could hook ArticleFileManager::handleUpload to run an external scan on a file.

github.com

pkp/ojs/blob/ojs-stable-2_4_8/classes/file/ArticleFileManager.inc.php#L489



	/**
	 * PRIVATE routine to upload the file and add it to the database.
	 * @param $fileName string index into the $_FILES array
	 * @param $fileStage int identifying file stage (defined in ArticleFile)
	 * @param $fileId int ID of an existing file to update
	 * @param $overwrite boolean overwrite all previous revisions of the file (revision number is still incremented)
	 * @return int the file ID (false if upload failed)
	 */
	function handleUpload($fileName, $fileStage, $fileId = null, $overwrite = false) {
		if (HookRegistry::call('ArticleFileManager::handleUpload', array(&$fileName, &$fileStage, &$fileId, &$overwrite, &$result))) return $result;

		$articleFileDao =& DAORegistry::getDAO('ArticleFileDAO');

		$fileStagePath = $this->fileStageToPath($fileStage);
		$dir = $this->filesDir . $fileStagePath . '/';

		if (!$fileId) {
			// Insert dummy file to generate file id FIXME?
			$dummyFile = true;
			$articleFile =& $this->generateDummyFile($this->article);

Under an appropriate configuration, it shouldn’t be possible to execute uploaded submission files on the server, so this doesn’t substantially affect server security. It could, on the other hand, guard an editorial user against downloading a malicious word or PDF document.

newone · February 3, 2017, 4:39am

Yes, it would be to guard an editorial user from downloading a malicious word or PDF document.

I am not sure how to do this. Does it involve changing a code?

_newone

newone · February 3, 2017, 4:50am

@asmecher

Yeah, they become yours, but those submissions does not show up under your submissions after the user is merged. Additionally, these are submissions that are still in progress, meaning that the user did not finish his/her submission. The submissions I am referring to were never finished.

It would be nice if there is a way that journal managers can see incomplete submissions from all the authors in the system. Sometimes, people do not finish their submission for whatever reason, and the submission just sits there. It would be nice if editors will be able to see incomplete submissions.

_newone
It

ctgraham · February 3, 2017, 1:16pm

We just ran into the issue of not seeing incomplete submissions from the editorial side of 2.4.x yesterday. We wanted to delete a duplicate section, but there were two incomplete submissions using that section. The editorial staff were unable to find those submissions, but could login as the authors to see and correct the section assignment.

It would require a code change (preferably, a plugin) to hook the ArticleFileManager::handleUpload event to perform a dynamic virus scan of submitted files. A similar (but not identical) question was discussed here:

asmecher · February 3, 2017, 4:43pm

Hi all,

I think the problem is that incomplete submissions won’t show up unless the account has an “Author” role. Note that this is improved with OJS 3.x – we no longer have such a fixed set of interactions defined by the user’s role.

If a common use case for the “Merge Users” tool is to delete spammers – and I think it is – then perhaps it’s worth adding an option to delete their assets entirely, rather than reattributing them?

Regards,
Alec Smecher
Public Knowledge Project Team

newone · February 10, 2017, 10:17pm

Thanks on the comment about checking uploaded files.

If something is developed or if you have the required changes that needs to happen, do let me know.

-newone

newone · February 10, 2017, 10:28pm

In OJS 2.4.X, you have to have an “Author” role to to be able submit. So, the account that have incomplete submissions always have “author” roles.

The merge tool is a way to remove users and spammers. Sometimes, people want to delete their account, and it would be cool to have a way to see if they have open submissions or any other tasks that they have not completed, like review at the time their account is removed.

I have not used OJS 3 (no subscription in place), but it would be important to have this feature.

-newone