[DONE][security improvement] Possible edge case security issues

[AD7six]

LITTLE NOTICE FROM PHP-MVC AUTHOR:
Please don't panic, this ticket here shows some security issues that are indeed real, but php-mvc is on the same level of security like most mainstream PHP scripts in the world, like Wordpress, lots of CMS and major frameworks! The cases shown here are real, but these security "holes" can be found in most PHP scripts/installations in the world, including lots of popular sites.

These security issues will be fixed within the next 14 days by a simple movement of the index.php and .htaccess changes, so you can update to the fixed version easily.

Big thanks to @AD7six for the good information!

---

This project is insecure

Following on comments made to this SO question. If no other action comes from this ticket it would be wise to clarify beyond doubt the purpose of the repository as currently:

• "it's a training thing for people to get into the very basics of MVC"
• "People who [use this repository for real projects] should know what they do"

Which is quite contradictory, it's for beginners who know what they are doing.

The readme makes no mention of that; It is unfair to users who may choose this project for real applications to unwittingly find that the repository they have based their project on is infact fundamentally unsafe.

---

Consider the following actions:

ssh server
git clone https://github.com/panique/php-mvc.git
vim php-mvc/application/config/config.php

Resulting in:

$ tree php-mvc
|-- CHANGELOG.md
|-- README.md
|-- _tutorial
|   |-- donate-with-paypal.png
|   |-- tutorial-part-01.png
|   |-- tutorial-part-02.png
|   |-- tutorial-part-03.png
|   |-- tutorial-part-04.png
|   `-- tutorial-part-05.png
|-- application
|   |-- _install
|   |   |-- 01-create-database.sql
|   |   |-- 02-create-table-song.sql
|   |   `-- 03-insert-demo-data-into-table-song.sql
|   |-- config
|   |   |-- config.php
|   |   `-- config.php~ # <---- an editor file.
|   |-- controller
|   |   |-- home.php
|   |   `-- songs.php
|   |-- libs
|   |   |-- application.php
|   |   `-- controller.php
|   |-- models
|   |   |-- songsmodel.php
|   |   `-- statsmodel.php
|   `-- views
|       |-- _templates
|       |   |-- footer.php
|       |   `-- header.php
|       |-- home
|       |   |-- example_one.php
|       |   |-- example_two.php
|       |   `-- index.php
|       `-- songs
|           `-- index.php
|-- composer.json
|-- index.php
`-- public
    |-- css
    |   `-- style.css
    |-- img
    |   `-- demo-image.png
    `-- js
        `-- application.js

All application files are web accessible

Absolutely all files are accessible in a php-mvc project. For all not-php files that means the source can be read directly. Example:

$ curl http://example.com/php-mvc/composer.json
{
    "name": "panique/php-mvc",
    "type": "project",
    "description": "A simple PHP MVC boilerplate",
...

The problem is not limited to !php files, as most editors generate a tmp/swap/backup file any file that's edited on the server (or has noise uploaded - if it's deployed via ftp) is also accessible. In the example I gave a backup/swap file was generated for the config file, that file's contents are also now browsable:

$ curl http://server/php-mvc/application/config/config.php~
<?php

/**
 * Configuration
 *
 * For more info about constants please @see http://php.net/manual/en/function.define.php
 * If you want to know why we use "define" instead of "const" @see http://stackoverflow.com/q/2447791/1114320
 */

/**
 * Configuration for: Error reporting
 * Useful to show every little problem during development, but only show hard errors in production
 */
error_reporting(E_ALL);
ini_set("display_errors", 1);

/**
 * Configuration for: Project URL
 * Put your URL here, for local development "127.0.0.1" or "localhost" (plus sub-folder) is fine
 */
define('URL', 'http://127.0.0.1/php-mvc/');

/**
 * Configuration for: Database
 * This is the place where you define your database credentials, database type etc.
 */
define('DB_TYPE', 'mysql');
define('DB_HOST', '127.0.0.1');
define('DB_NAME', 'php-mvc');
define('DB_USER', 'root');
define('DB_PASS', 'mysql');

The code is also browsable via the .git folder

But the bad news doesn't stop there. You can also browse the .git folder if it is "deployed" with the application (if either the project is a checkout on the server, or the .git folder is uploaded via ftp):

$ curl http://example.com/php-mvc/.git/config
[core]
    repositoryformatversion = 0
    filemode = true
    bare = false
    logallrefupdates = true
[remote "origin"]
    fetch = +refs/heads/*:refs/remotes/origin/*
    url = https://github.com/panique/php-mvc.git
[branch "master"]
    remote = origin
    merge = refs/heads/master

In and of itself this may disclose sensitive information. Getting a valid response means that by looking around you can browse/download the whole git repo:

$ curl http://example.com/php-mvc/.git/packed-refs
# pack-refs with: peeled 
2c7c8b01ea5904098bc5a7a22a93781082c8eacc refs/remotes/origin/master
14b1162512b50d3ea0a71f8e0c501a7a30445bae refs/remotes/origin/develop

etc.

Solution

It is trivial to prevent this; the only side effect being that the public folder is the only folder that is web accessible.

• Move /index.php to /public/index.php

• Move /.htaccess to /public/.htaccess

• create a new root .htaccess file with the following contents:

  <IfModule mod_rewrite.c>
      RewriteEngine on
      RewriteRule    (.*) public/$1    [L]
  </IfModule>
  

While I have no personal interest in using this project - I'd be grateful if you could address these problems to prevent inexperienced users trying to use it and inadvertently disclosing their application files to the public or worse losing their data etc.

In addition I recommend taking a look at projects such as h5bp's apache config as it addresses these and many more common problems out of the box.

[panique]

Hey, first a big thanks for this, but some things need to be clarified:

1. The htaccess rules etc used here are EXACTLY the same as in nearly every other framework or major piece of PHP software, like wordpress.

2. The source code of any .php files is NOT viewable. I can not reproduce this. If .php files would be easily downloadable, well, then the entire PHP itself would have a BIG problem. How many frameworks do you know that DON't work with a public/ folder ? Nearly all of them, and that for years. doing a curl on the config (with ~ or without) simply doesn't return anything because you get a forbidden error. But let's discuss this further, maybe you are using this with special server settings that create this situation.

Just to redefine the situation: When you are editing a file with an editor right on the live-server (which nobody would ever really do) then the EDITOR (!) might put a temporarely copy of the edited file inside the same folder (common situation on every OS). These files might be readable from the outside, as they are not .php files and are therefore delived as static files (like. txt or .jpg) and not interpreted by Apache as PHP. Interesting case, but really totally out of scope and really edge, as you'll need the name of the file and hit the server in the exact moment this temp copy exists.

Please correct me if I'm wrong here.

3. That your .git folder is accessable UNDER CERTAIN CIRCUMSTANCES is a common problem, and has absolutly nothing to do with this project itself. People who use git to deploy usually know what they do.

4. Yes, the json file is browseable (if you deployed it (!)), but you can see that in lots of projects on the web.

Also, for all the people who are stuck with the above example:
This example uses VIM, an editor that is horrible for beginners, as it may lock you out of your console. Unless you know vim, use nano as a qucik and good alternative.

@ALL: This is a super-basic MVC folder/file structure, and it's clearly defined as this. It's out of the scope of this script to teach people server security stuff.

@ALL: Can you people please try this and give feedback ?

@AD7six Again, thanks for the notice and the excellently written tutorial, I deeply thank you for that (even if I cannot reproduce / agree totally). I'll look deeply into that if there's time, and in general I agree that as a script author there's a certain responsibility to prevent every special security issue, even if it's not really in the scope of the script.

[GrahamCampbell]

@panique Surely the simplest solution is just to stick index.php and .htaccess in the public folder, and get people to point apache/whatever to that folder instead. I assume we're not doing this to support people on shared hosts who can't change the base folder.

[AD7six]

1. The htaccess rules etc used here are EXACTLY the same as in nearly every other framework or major piece of PHP software, like wordpress.

Your project is not wordpress, and wordpress is far from the pillar of web security.

2. The source code of any .php files is NOT viewable

I don't think you understand the ticket.

I didn't say the php files show their contents - I said [all files] are accessible. PHP files are vulnerable only if there are editor swap files (a relatively common problem).

maybe you are using this with special server settings that create this situation.

Absolutely not. stock apache on a test server. Alternatively point at any public site using php-mvc and it's quite likely an exploit or private information can be extracted.

The above example is a hard-edge case and totally out of scope of the project.

Er.. no, definitely not a corner case. See above reference.

3. That your .git folder is accessable ...

Well no that's the whole point here. The people who deploy using git are the same people who don't know what they are doing or aren't aware of the risks.

4. Yes, the json file is browseable (if you deployed it (!)), but you can see that in lots of projects on the web.

Indeed you can, that doesn't make it any less of a problem. It's one file that contains semi-sensitive information - all files are accessible.

use nano as a qucik and good alternative.

Nano also creates swap files...

I agree that as a script author there's a certain responsibility to prevent every special security issue

Here is where we disagree. I've pointed out (very) common pitfalls that have significant consequences - and you refer to them as "every special security issue".

[panique]

@AD7six Thanks for the excellent feedback, here's my part:

1. Okay, but let's be honest: this is just a tiny script i've written in my freetime and shared it on github, never asking for popularity. Does this really needs to be MORE secure than Wordpress, which is the most user "user-facing" web software in the world, used by NASA and NY Times etc. ? But you're right I think, if there's the possibility to make things better easily, then we should make them better.

2.) You got me, I indeed misunderstood it. For everbody else reading this and also didn't got it: @AD7six meant that the files can be run via direct access, NOT READ. You will not seee any code. Btw I COULD NOT reproduce this, I get an forbidden error, but I'm trying different setups to reproduce. More later...

The editor swap file problem: Interesting, didn't know that, but isn't this a general web server problem, something that will probably affect major parts of the entire web ? Good to see this solved with moving index.php!

1. + 4) Okay, let's fix this.

Nano.) No, you misunderstood me: I'm not talking about swap files, I simply meant that VIM might be hard to use to people who want to try out your given instructions to reproduce the problem! Most people using vim for the first time are stuck within the editor as it's no obvious how to leave :)

Responsibility:) Okay, I see! The swap file and .git problems are totally new to me, and I bet most PHP developers also never heard of them. I still would consider accessing swap files of temporarly edited files as a special case, but you are the security expert here, so I trust you :)

To sum this up: Again a big thanks from me, I'll implement the above fix in the next days or weeks.

[panique]

@AD7six Wouldn't it be simpler to change
https://github.com/panique/php-mvc/blob/master/application/.htaccess
from

<Files *.php>
    Order Deny,Allow
    Deny from all
</Files>

to

<Files ~ "\.(htaccess|php)$">
order allow,deny
deny from all
</Files>

to prevent access to any .php files in the application folder and deeper ? The according main .htaccess in the project's root would look like this:

# Necessary to prevent problems when using a controller named "index" and having a root index.php
# more here: http://stackoverflow.com/q/20918746/1114320
Options -MultiViews

# turn rewriting on
RewriteEngine On

# RewriteBase is skipped here

RewriteCond %{REQUEST_FILENAME} !-d
RewriteCond %{REQUEST_FILENAME} !-f
RewriteCond %{REQUEST_FILENAME} !-l

RewriteRule ^(.+)$ index.php?url=$1 [QSA,L]

[AD7six]

<Files ~ "\.(htaccess|php)$">

Well, genuine but rhetorical question: Will it address or change anything I put in this ticket?
What do you think that Files block will do?

I think right now you are still in some permutation of not understanding the ticket/"there isn't really anything wrong"/"that's not my responsibility".

I'm willing to help you if you want the help; I can just PR the changes, they really are not significant. Alternatively I predict based on his github activity that @GrahamCampbell has the same opinion/insight (though what I proposed does not mean users must be able to change the document root, it'll work whether they do or they don't).

If you want cannot-fail steps to reproduce what's in this ticket^[*] yourself do the following:

ssh server
git clone https://github.com/panique/php-mvc.git
cp php-mvc/application/config/config.php php-mvc/application/config/anything.anyextension
cp php-mvc/application/config/config.php php-mvc/anything.anyextension
cd php-mvc; git commit -a -m "findme"

"anything.anyextension" means exactly that; it shouldn't matter what a file is called in your application folder, nothing inside it is intended to be directly accessed; nothing outside the public folder should be directly accessed.

If any of these curl commands or similar permutations return a response this ticket isn't resolved:

• curl http://example.com/php-mvc/application/config/anything.anyextension
• curl http://example.com/php-mvc/anything.anyextension
• curl http://example.com/php-mvc/.git/logs/HEAD

[*] assuming no security rules are in the apache config or any .htaccess files in parent directories.

[panique]

Yes, <Files ~ "\.(htaccess|php)$"> is a bad example, but I haven't found out how to block entire access to anything, something like <Files ~ "\.(EVERYTHING)$"> is what i mean.

I fully understand the problem, the given example was just bad. So again the question: Is blocking access to EVERYTHING in application an alternative (even is making public the project's public root is the better solution) ?

[AD7six]

I haven't found out how to block entire access to everything

You can just remove the files block, i.e. the full .htaccess file's contents would be:

order deny,allow
deny from all

Is blocking access to EVERYTHING in application an alternative

Well it's much, much better but IMO no, it's not an alternative - see my previous comment ("if any of these curl commands ...")

[adamholte]

I just implemented the change AD7six proposes and it was pretty simple to change a few things. I created some constants in the index file for easier reference to the application folders, and I even removed the .htaccess from the application folder and was unable to get to any of the files he points out. I have a small installation in a subfolder on a shared host and all is working well.

It may not be your place to teach everyone about everything, but it is a simple security measure to make the web a better place.

[panique]

@adamholte Okay, thanks for the feedback, but I'm not sure if an installation on shared hosting is representative here. I'll integrate the fix generally in the next few days. Fixing is not the big part, but the testing may take some time, you know...

[panique]

Btw this is a bad solution for local development, especially for beginners and learners who simply want to setup a basic mvc application. Currently people can download the script into their web root or a subfolder and are ready to go. But with the above fix they will need to setup a vhost which makes things much more complicated, especially when working locally.

Let's discuss this further, please! Would be good to hear the opinions of others. And how could a possible solution look like ? Unfortunatly .htaccess is not able to block access to folders (!?), just files or file-endings, therefore a "block everything except index.php and /public"-rule is not possible.

The solution should do the following:

• block any access to application/ (main app folder)
• block any access to vendor/ (composer-loaded dependencies)
• block any access to composer.json
• block any access to composer.lock

[Mathlight]

Just tosing my 2 cents in...

Currently people can download the script into their web root or a subfolder and are ready to go. But with the above fix they will need to setup a vhost which makes things much more complicated, especially when working locally.

This isn't much of an problem right? I've edited my wamp server to start in the public directory. But you can also change the URL in the config file to http://localhost/public/ and then navigate to it in your browser.

You can also use this piece of .htaccess in your root to auto navigate them to the public folder:

RewriteEngine On
RewriteRule ^$ /public [L]

( found here by SO )

[panique]

@Mathlight Not sure if we are talking about the same thing, but setting up a vhost ist way too much for most peole working with the classic local development tools, especially for the people working on Windows. No judgement here, but it's really not that easy for beginners. But maybe we can add tiny tutorials on how to do that in WAMP etc. ... Problem here: This will rewrite all access to root to /public, but most people have SEVERAL projects inside their root folder (that's at least my experience with people working with local development tools), so rewriting all root-access to /public (with .htaccess or via vhost) is not a solution here.

[Mathlight]

@panique I wasn't talking about vhost indeed, because i know that i can be hard for an newbie to set them up properly ( although Google is an good friend of everybody... )

And sorry for the misunderstanding, but when i mean root, i mean root of the project. So in localhost/php-mvc/[root]
that will become localhost/php-mvc/public then, right?

It's just my 2 cents, do with it whatever you like... And keep up the good work ( i freaking love this skeleton! )

[panique]

@Mathlight Thanks :)

[panique]

This is now part of the develop branch. Big thanks! coming to master when tested widely.

[panique]

@AD7six FYI: The extremely popular micro-framework Slim (and most others) put the index.php also in the root folder, together with a htaccess file that routes everything to index.php. Is this less secure than the now-implemented solution here (route everything to /public/index.php) ?

[PeeHaa]

Yes it is. .htaccess can be disabled, nginx can be used, server can be misconfigured. Also popular says exactly nothing about quality.