Restrict author to upload only doc/docx/pdf file for article submission

Software Support
1

Dear OJS Support,

I was hacked yesterday because someone with other intentions rather then participating in our Journal was able to upload .phtml files, .cer files and .pl files… how can i restrict author to upload only doc/docx/pdf file for article submission? This is a serious issue.

Thanks for your time, keep OJS awesome!
Miguel

2

Hi

A simple way to make sure that it does not happen again is to follow the installation instructions and move your files folder outside the webroot, meaning that it is not a subdirectory of you OJS installation. Also remember to update your config.inc.php and add the correct path there.

The authors can still upload phtml files, but they can not use them to hack your system, because the files are not accessible.

See: PKP Applications and Security

3

Hi!

Many thanks for your answer!
But is this feasible now? My OJS website is running for 4 or 5 years now…

Thank you,
Miguel

4

You can move the files folder also in old sites. I moved ours a couple of years ago and the site had been online for 7 years by then. You just need to make sure the path is correct in config.inc.php.

The fact that you have not been hacked before is pure luck. I think hackers have been more active with OJS since 2015.

5

Thanks for your reply ajnyga!

I’m on it, I didn’t knew I could do this, i thought that file URL’s would be stored in the database.
Just by curiosity, would it be possible if OJS would only accept .doc, docx and pdf files?

Best regards,
Miguel

6

Just remember to always do backups!

I guess you could edit the code around here to make that possible: https://github.com/pkp/pkp-lib/blob/master/classes/file/FileManager.inc.php#L528

But even then the best protection is to move the files folder outside the web root.

7

Done! Files outside public_html.
Let’s hope it’s enough :).

Thanks again and if you come by Portugal please let me know, I’ll buy you a beer!

Best regards,
Miguel

8

Beer in Portugal sounds good, it is dark, rainy and cold in Finland :smiley:

We still get phtml uploads from time to time, but the hacker can not use the files to their advantage now that the files folder is not directly accessible. So I just search for them and remove them from time to time.

One thing you should check of course is that the hacker did not leave any backdoors, I mean edit some OJS core file. This could be done for example with a diff. You can search the forum for more details.

9

Hi all,

We have two reasons for not limiting uploads to a certain type in order to avoid this kind of attack:

  • If your files directory is publicly accessible, then it’s not just executables that you need to worry about – your submission files can be downloaded by anyone who can guess the URLs, potentially exposing unpublished research.
  • Using a block-list approach (whereby we list types of files that should not be allowed), it’s impossible to know what your server might potentially execute. Using a permit-list approach (whereby we list types of files that should be allowed), we drastically limit submission types that might well be valid. And some of the “risky” (executable) types do have legitimate uses in a journal, especially as publications start to include code e.g. for statistical analysis even outside technical fields.

Moving your files directory outside the public_html directory will solve both problems – legitimate scholars and would-be hackers alike are free to upload .phtml files (etc)., but they pose no risk if they are malicious.

Regards,
Alec Smecher
Public Knowledge Project Team

10

Many thanks for your clarification Alec!
We had a complicated weekend cleaning “false” submissions.

I placed the files folder outside public_html and created a .htaccess with the following:
RewriteRule ^files/.*.(phtml|cer|sm|php)$ - [F,L,NC]

Will this help?

Warm regards from Portugal,
Miguel

11

Hi @miguelbraga,

If your files directory is outside public_html, then your web server should have no reason to access it (except through OJS, which is a different matter), so the .htaccess file won’t ever get looked at (and is unnecessary). But if the .htaccess file were necessary, e.g. if you’d left files_dir within public-html, then you would probably need to be concerned about other file types like .pl, .py, and whatever else your server considered executable.

Regards from rainy Vancouver,
Alec Smecher
Public Knowledge Project Team

12

Thanks, and yes the folder is now outside public_html!
No problem occurred during today so I bet that there’s a frustrated hacker somewhere in the world :P!

P.S. Send the rain to Portugal, we are having a serious drought (no rain in the past 4 months or more…).

Regards,
Miguel Braga
Daughter Entertainer and Trail Runner :slight_smile:

13

How about restricting the uploads to a certain type of file for quality control purposes?

Some journals may only accept doc/docx files because that’s how their workflow works and anything other than that may slow down the process.

I’ve seen many cases where the author uploads a PDF file and the journal needs to request the Word file to the author and that could delay the process.

Also restricting file extensions for images to avoid receiving certain types of low quality image formats is a way to ensure quality and remove some of the workload from the journal staff (since the system will be doing that check, instead of the journal’s team).

14

Hi @alexxxmendonca,

I’d recommend putting that in the submission checklist for the author to see during the submission process – but yes, if authors aren’t following those instructions, this could be enforced with the use of a little bit of plugin code.

Regards,
Alec Smecher
Public Knowledge Project Team

15

Want to point a hook to use here, I actually looked for a hook for this last spring but recall that I could not find one. If you point a hook, I could prepare a plugin.

16

Our journal works only with doc/docx because of the nature of the research papers, a plugin that restricts filetypes would be awesome! Dear OJS developers please make this happen!!! Beer is on me ;)…

Regards,
Miguel

17

Hi @ajnyga,

Did you have a look at any of the form class hooks? These use introspection to determine the name of the subclass. You might need to temporarily add an error_log($hookName); to the hook manager in order to more easily identify it.

Regards,
Alec Smecher
Public Knowledge Project Team

18

No, I think I looked at the upload related functions. But I did the error_log trick now and here is a list of hooks that are called when you upload a file (select the file from you computer and upload it to the first stage of the three step form).

[24-Nov-2017 22:58:09 Europe/Helsinki] LoadComponentHandler
[24-Nov-2017 22:58:09 Europe/Helsinki] submissionfiledao::_Constructor
[24-Nov-2017 22:58:09 Europe/Helsinki] articledao::_Constructor
[24-Nov-2017 22:58:09 Europe/Helsinki] authordao::_Constructor
[24-Nov-2017 22:58:09 Europe/Helsinki] submissiondao::_getbyid
[24-Nov-2017 22:58:09 Europe/Helsinki] dao::_getdataobjectsettings
[24-Nov-2017 22:58:09 Europe/Helsinki] ArticleDAO::_fromRow
[24-Nov-2017 22:58:09 Europe/Helsinki] submissionfilesuploadform::Constructor
[24-Nov-2017 22:58:09 Europe/Helsinki] submissionfilesuploadform::readuservars
[24-Nov-2017 22:58:09 Europe/Helsinki] submissionfilesuploadform::readuservars
[24-Nov-2017 22:58:09 Europe/Helsinki] genredao::_Constructor
[24-Nov-2017 22:58:09 Europe/Helsinki] genredao::_getbyid
[24-Nov-2017 22:58:09 Europe/Helsinki] dao::_getdataobjectsettings
[24-Nov-2017 22:58:09 Europe/Helsinki] GenreDAO::_fromRow
[24-Nov-2017 22:58:09 Europe/Helsinki] usergroupdao::_useringroup
[24-Nov-2017 22:58:09 Europe/Helsinki] submissionfilesuploadform::validate
[24-Nov-2017 22:58:09 Europe/Helsinki] genredao::_getbyid
[24-Nov-2017 22:58:09 Europe/Helsinki] dao::_getdataobjectsettings
[24-Nov-2017 22:58:09 Europe/Helsinki] GenreDAO::_fromRow
[24-Nov-2017 22:58:09 Europe/Helsinki] submissionfiledaodelegate::_Constructor
[24-Nov-2017 22:58:09 Europe/Helsinki] submissiondao::_getbyid
[24-Nov-2017 22:58:09 Europe/Helsinki] dao::_getdataobjectsettings
[24-Nov-2017 22:58:09 Europe/Helsinki] ArticleDAO::_fromRow
[24-Nov-2017 22:58:09 Europe/Helsinki] submissionfiledaodelegate::_insertobject
[24-Nov-2017 22:58:09 Europe/Helsinki] genredao::_getbyid
[24-Nov-2017 22:58:09 Europe/Helsinki] dao::_getdataobjectsettings
[24-Nov-2017 22:58:09 Europe/Helsinki] GenreDAO::_fromRow
[24-Nov-2017 22:58:09 Europe/Helsinki] usergroupdao::_getbyid
[24-Nov-2017 22:58:09 Europe/Helsinki] dao::_getdataobjectsettings
[24-Nov-2017 22:58:09 Europe/Helsinki] UserGroupDAO::_returnFromRow
[24-Nov-2017 22:58:09 Europe/Helsinki] pkpuserdao::_getbyid
[24-Nov-2017 22:58:09 Europe/Helsinki] dao::_getdataobjectsettings
[24-Nov-2017 22:58:09 Europe/Helsinki] UserDAO::_returnUserFromRowWithData
[24-Nov-2017 22:58:09 Europe/Helsinki] submissionfiledaodelegate::getLocaleFieldNames
[24-Nov-2017 22:58:09 Europe/Helsinki] submissionfiledaodelegate::getAdditionalFieldNames
[24-Nov-2017 22:58:09 Europe/Helsinki] dao::_updatedataobjectsettings
[24-Nov-2017 22:58:09 Europe/Helsinki] submissiondao::_getbyid
[24-Nov-2017 22:58:09 Europe/Helsinki] dao::_getdataobjectsettings
[24-Nov-2017 22:58:09 Europe/Helsinki] ArticleDAO::_fromRow
[24-Nov-2017 22:58:09 Europe/Helsinki] submissionfileeventlogdao::_Constructor
[24-Nov-2017 22:58:09 Europe/Helsinki] Request::getRemoteAddr

The submissionfilesuploadform hooks seemed to be useless, or at least I could not find any trace of the uploaded file in them.

However, in submissionfiledaodelegate::_insertobject you get

[24-Nov-2017 23:35:45 Europe/Helsinki] Array
(
    [0] => INSERT INTO submission_files
				(revision, submission_id, source_file_id, source_revision, file_type, file_size, original_file_name, file_stage, date_uploaded, date_modified, viewable, uploader_user_id, user_group_id, assoc_type, assoc_id, genre_id, direct_sales_price, sales_type)
				VALUES
				(?, ?, ?, ?, ?, ?, ?, ?, '2017-11-24 23:35:45', '2017-11-24 23:35:45', ?, ?, ?, ?, ?, ?, ?, ?)
    [1] => Array
        (
            [0] => 1
            [1] => 17
            [2] => 
            [3] => 
            [4] => image/svg+xml
            [5] => 63108
            [6] => logo.svg
            [7] => 2
            [8] => 1
            [9] => 1
            [10] => 2
            [11] => 
            [12] => 
            [13] => 1
            [14] => 
            [15] => 
        )

    [2] => 
)

Here you can see the file type, but I think that when you get here, the file is already uploaded to the server. So you would have to write a plugin that checks this data and if there is a problem, would remove the already uploaded file.

That is probably fairly easy to do, however, the problem is how to give feedback to the user that the file type is not what is expected.

19

@miguelbraga see, https://github.com/ajnyga/allowedUploads/releases/tag/OJS3.1.0.1

That plugin should enable you to limit the allowed file extensions for submission files. NOTE! This is not a security plugin so make sure that your files directory is not a subdirectory of your OJS installation.

@asmecher it turned out to be much easier than I expected after I realized that I could get the filename from the request when calling the form::validate hook. The only thing which is problematic is that the error messages that are shown in the small modal windows get “stuck” and only show up when the upload process is finished. This is something that is not, in my opinion, related to this plugin alone. I have seen a lot of functions in OJS where those messages get stuck and then show up later.

20

AWSOME!!! My friend dinner is on me, you will love Portuguese food! :slight_smile: If you need my humble designer help help please do ask. Bring @asmecher to sunny Portugal too.