Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change web root from application root directory for security #1832

Closed
kardeiz opened this issue Sep 19, 2016 · 7 comments
Closed

Change web root from application root directory for security #1832

kardeiz opened this issue Sep 19, 2016 · 7 comments
Labels
Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day.

Comments

@kardeiz
Copy link

kardeiz commented Sep 19, 2016

I'm a little concerned about the structure of the OJS application (both versions 2 or 3). In standard installations, the base OJS directory is the HTTPD DocumentRoot, and any OJS file can be requested by an HTTP client (unless protected from such access, as config.inc.php is).

Most major PHP applications that I've seen or worked with handle this by doing one of the following:

  1. Make a subfolder (e.g., ./Web or ./public) the DocumentRoot, with index.php within that folder requesting application code from private folders (e.g., ./app or ./src) as necessary.
  2. Include a detailed .htaccess that prevents HTTPD from reading most PHP files and other special locations.

Without digging into it too much, I see a few major issues with how OJS is structured:

  1. While (by default) the only file that really contains sensitive information is config.inc.php, and that file is protected from access (which is good!), is it really safe to allow access to other PHP files? Is every file guaranteed to be side-effect free (that is, will any code be run when a PHP file is requested that will affect the operation of the system)? Is every file guaranteed to not leak any information? If the code errors out (e.g., due to failed imports or something else), it will probably leak some details about the file's location on the server (or more!).
  2. It may not be clear to OJS users that any customizations of PHP or TPL files may be directly visible to the Web. While it is not especially likely that users would add sensitive information here, it is possible that they might.
  3. Users who are accustomed to the public Web subfolder strategy listed above, and who skim over installation instructions, might put their files folder in the base directory without realizing that it is web-accessible. I suspect this is why so many people are hit by this issue: http://forum.pkp.sfu.ca/t/security-issue-hacking-via-submission-in-ojs-2-4-8/16382/2.
@asmecher
Copy link
Member

@kardeiz, all of the PHP code is safe for direct access should someone attempt that -- we define classes in .inc.php files but do not execute procedural code.

Including an .htaccess file is certainly an option, but not all servers are configured to respect them, so I'd worry about a false sense of security.

As for a Web or public subdirectory, it's not clear to me how this would work with commodity (cheap) web hosts. Typically there's a public_html directory, and PHP-level tools like open_basedir may restrict the application from including e.g. PHP scripts from above that directory.

Thanks for the feedback -- if we're able to come up with improvements that are broadly applicable, I'm all ears. Our community's variety of deployment environments is ridiculously diverse, so this can be difficult.

@kardeiz
Copy link
Author

kardeiz commented Sep 19, 2016

@asmecher, thanks for the fast response!

I admit I hadn't thought about not being able to change the DocumentRoot. But, assuming that all files that needed to be publicly accessible were moved within a Web folder, wouldn't it be the case that:

  • users with shared webhosts would be in the exact same situation they are now, except with a Web component in their URLs (that could easily be hidden with an .htaccess file if necessary)
  • everyone able to change their DocumentRoot to ojs/Web would have a more secure setup (arguably)

Here is some information on how Symfony recommends working with shared hosting. And phpScheduleIt/Booked is an application I've worked with that has a public Web subdirectory that supports living in a public_html folder.

@asmecher
Copy link
Member

I'd worry that with Symfony's approach incautious users would still dump the application into their web root and access it via the web subdirectory. The risk isn't so much of direct access to .inc.php scripts that are intended for inclusion rather than execution -- it's that users aren't cautious about the file storage area. So far the best suggestion (IMO) is to include an .htaccess file restricting direct access to the files directory -- but on the downside not all servers will support .htaccess, and we would be encouraging all users to put their files_dir within the web root. I'm not sure the gain is worth the risk.

@WaDelma
Copy link
Contributor

WaDelma commented Sep 22, 2016

Couldn't OJS installation script check if files_dir is accessible from outside and error out if that is the case?

@kardeiz
Copy link
Author

kardeiz commented Sep 22, 2016

Perhaps I shouldn't have mentioned the files_dir safety issue—it's an important issue, and probably a related one, but not the main focus of my posting.

The risk isn't so much of direct access to .inc.php scripts that are intended for inclusion rather than execution [...]

But there are (by default in OJS 3.0) 818 plain .php (not .inc.php) files that may or may not contain procedural code. While you may be sure that the PHP written by you and the PKP/OJS team is all encapsulated in classes, what about all of the third-party code used by OJS? What about users' local modifications? It seems really hard to guarantee that no PHP file will contain procedural code. It also seems very possible that information could leak on error when a PHP file is requested directly.

I know that Wordpress and some other projects have a similar model, but it seems like the current consensus is that it is best practice to move as much code as possible outside of the web root. As an infrequent PHP user, the idea that any library/model/config code can be executed on demand (even if it doesn't really do anything), is really strange to me.

As noted above, I think re-organizing the codebase (distinguishing between public and private/included PHP files) would be helpful for those who could take advantage of it, and would not be harmful for those who could not.

@asmecher
Copy link
Member

@kardeiz, you make a decent point. I'll leave this issue open for some exploration/consideration, but won't likely be able to devote time to it in the near future. We occasionally hold code sprints, and this might be the kind of thing that's suitable for toying with during one of those.

@TobiasWantzen
Copy link

@asmecher If you want to see, how a symphony based CMS manages this part, take a look at www.contao.org, which changed to a symphony application since version 4. The only caveat: The installation process is not as easy as it was.

@NateWr NateWr changed the title [OJS] Safety/security of OJS application structure Change web root from application root directory for security Nov 10, 2021
@NateWr NateWr added the Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day. label Nov 10, 2021
@pkp pkp locked and limited conversation to collaborators Jul 25, 2022
@NateWr NateWr converted this issue into discussion #8114 Jul 25, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day.
Projects
None yet
Development

No branches or pull requests

5 participants