Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 538822 - app-eselect/eselect-php[apache2] marks all files containing ".php" for execution instead of "*.php"
Summary: app-eselect/eselect-php[apache2] marks all files containing ".php" for execut...
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: [OLD] Server (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: PHP Bugs
URL:
Whiteboard:
Keywords: PATCH
Depends on:
Blocks: 544560 545664
  Show dependency tree
 
Reported: 2015-02-04 16:05 UTC by Nico Suhl
Modified: 2016-03-10 00:48 UTC (History)
4 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
apache2 handlers for php files (apache2_php.diff,552 bytes, patch)
2015-02-04 16:06 UTC, Nico Suhl
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nico Suhl 2015-02-04 16:05:25 UTC
app-admin/eselect-php with apache2 flag creates /etc/apache2/modules.d/70_mod_php5.
This file allows the php interpreter to be used for all .php files handled by apache2.
Sadly in the current configuration all files _containing_ ".php" in their names are
processed by php. This leads to unwanted behavior and security risks.

The added patch ensures that only files ending with .php (and .php(5|s|html) and handled.

Reproducible: Always
Comment 1 Nico Suhl 2015-02-04 16:06:36 UTC
Created attachment 395534 [details, diff]
apache2 handlers for php files
Comment 2 Nico Suhl 2015-02-04 16:08:14 UTC
PHP suggests that configuration: http://php.net/manual/en/install.unix.apache2.php
Comment 3 Michael Orlitzky gentoo-dev 2015-02-05 15:45:03 UTC
Well, you're right, but changing it presents a bigger security risk.

First of all, the existing AddHandler config doesn't process files with ".php" anywhere in the name, only if the file has multiple extensions. So index.php.html would be processed, while index_.php_foo.html would not.

Second, I think this is a lot clearer than the regexp they give on php.net:

  <FilesMatch "\.(php|php5|phtml)$">
    SetHandler application/x-httpd-php
  </FilesMatch>

  <FilesMatch "\.phps$">
    SetHandler application/x-httpd-php-source
  </FilesMatch>

But the big problem is that if anyone was relying on the old behavior and we change it, their websites are going to start serving up the code to their PHP pages. That code can contain database passwords or other sensitive information.

There's also a risk to the current config. If you run a Wordpress site and you allow visitors to upload files, you want to prevent them from uploading a PHP file and then running it via www.example.com/.../uploads/exploit.php. On the server side, you may think you're preventing this by blocking files with a ".php" extension, but in fact ".php.txt" would also be executed. The AddHandler config works as documented, though, and to prevent that sort of thing you have to know how it works. So it isn't a huge problem. NB: It looks like I've made this mistake on at least one of my servers, so I'm not downplaying it.

So maybe it can be changed, but it would need a news item or something.
Comment 4 Nico Suhl 2015-02-09 21:40:47 UTC
This should definitively be changed as soon as possible. A lot of applications depend on checking only the file ending and I don't see anyone expecting this behavior.

A news item would be a good way to inform about this.
Comment 5 Sebastian Pipping gentoo-dev 2015-02-14 17:46:48 UTC
PHP team, any objections?

I would dare to fix this myself in a few days, otherwise.

Thanks!
Comment 6 Sebastian Pipping gentoo-dev 2015-02-14 18:04:00 UTC
Now fixed at https://wiki.gentoo.org/wiki/PHP#Apache_.28mod_php.29 .
Comment 7 Sebastian Pipping gentoo-dev 2015-02-15 22:55:38 UTC
PS: Since /etc/apache2/modules.d/70_mod_php5.conf is in config protection area, the admin needs to run etc-update (or one of its friends) manually to apply the fixed version.  To me that means:

 * Fixing the file in app-admin/eselect-php does not result in broken set-ups
   since the admin is forced to review and apply the change him- or herself.

 * That we do need a news item or GLSA or both to make people aware
   so they actually do run etc-update and do not discard the changes.
Comment 8 Sebastian Pipping gentoo-dev 2015-02-15 23:04:23 UTC
(In reply to Michael Orlitzky from comment #3)
> But the big problem is that if anyone was relying on the old behavior and we
> change it, their websites are going to start serving up the code to their
> PHP pages. That code can contain database passwords or other sensitive
> information.

Would it be possible to combine the regex SetHandler and a call to AddHandler sending "evil.php.png" to 404?  Is there a handler like that we could call?
Comment 9 Michael Orlitzky gentoo-dev 2015-02-23 02:57:55 UTC
(In reply to Sebastian Pipping from comment #8)
> 
> Would it be possible to combine the regex SetHandler and a call to
> AddHandler sending "evil.php.png" to 404?  Is there a handler like that we
> could call?

You could do another FilesMatch matching names with "php." in the middle of them. Then instead of SetHandler, just "Require all denied" or "Deny from all" (depending on the Apache version).

I wouldn't bother though: we have enough ancient fixes for ancienter hacks in our apache config. The news item should alert people, YOU NEED TO CHECK YOUR CONFIG TO MAKE SURE THIS ISN'T A PROBLEM. At that point, they can issue a simple `find` command and check those files themselves (they'd have to anyway, since they would be 404ing!).

The 404-handler could also break pages with funny names that weren't actually a problem before.
Comment 10 Sebastian Pipping gentoo-dev 2015-02-23 17:40:57 UTC
Alright.

For news item, are we aiming at a regular portage news item or for a GLSA?

Do we roll a patch before or after the news item release?
Comment 11 Michael Orlitzky gentoo-dev 2015-03-04 18:44:19 UTC
Since no one else has answered, I think we should prepare a new version of eselect-php, and then release the news item with a subject that mentions the new version (like e.g. the udev upgrade warnings).

The GLSAs aren't read by very many people so a regular news item makes more sense.
Comment 12 Sebastian Pipping gentoo-dev 2015-03-26 16:58:50 UTC
I have sent a portage news item to gentoo-dev for review now.

http://thread.gmane.org/gmane.linux.gentoo.devel/95282
Comment 13 Alex Legler (RETIRED) archtester gentoo-dev Security 2015-03-26 19:55:57 UTC
(In reply to Michael Orlitzky from comment #11)
> 
> The GLSAs aren't read by very many people so a regular news item makes more
> sense.

[citation needed]
Comment 14 Alex Legler (RETIRED) archtester gentoo-dev Security 2015-03-26 20:25:23 UTC
(In reply to Sebastian Pipping from comment #12)
> I have sent a portage news item to gentoo-dev for review now.
> 
> http://thread.gmane.org/gmane.linux.gentoo.devel/95282

And as such will be handled as hardening, not vulnerability.
Comment 15 Christian Ruppert (idl0r) gentoo-dev 2015-03-26 20:38:36 UTC
So this is actually a bug in Apache's AddHandler or what? Because even the Apache docs would be wrong then, the examples and also the description isn't clear then.
Comment 16 Sebastian Pipping gentoo-dev 2015-03-27 00:04:30 UTC
(In reply to Christian Ruppert (idl0r) from comment #15)
> So this is actually a bug in Apache's AddHandler or what?

Feature or bug, depending on ones view, yes.


> Because even the
> Apache docs would be wrong then, the examples and also the description isn't
> clear then.

Depending on the definition of "wrong", at many places they are, yes.
See https://blog.hartwork.org/?p=2669 .
Comment 17 Christian Ruppert (idl0r) gentoo-dev 2015-03-27 11:20:18 UTC
(In reply to Sebastian Pipping from comment #16)
> (In reply to Christian Ruppert (idl0r) from comment #15)
> > So this is actually a bug in Apache's AddHandler or what?
> 
> Feature or bug, depending on ones view, yes.
> 
> 
> > Because even the
> > Apache docs would be wrong then, the examples and also the description isn't
> > clear then.
> 
> Depending on the definition of "wrong", at many places they are, yes.
> See https://blog.hartwork.org/?p=2669 .

Well, so even Debian, SLES and likely others do (ab)use AddHandler then. Isn't it worth a CVE then? It first looked like it's a Gentoo issue but from what I've read so far it's not.
Comment 18 Sebastian Pipping gentoo-dev 2015-03-27 14:12:05 UTC
(In reply to Christian Ruppert (idl0r) from comment #17)
> Well, so even Debian, SLES and likely others do (ab)use AddHandler then.
> Isn't it worth a CVE then? It first looked like it's a Gentoo issue but from
> what I've read so far it's not.

I agree about the "not Gentoo only" part.
I don't object to taking this to oss-security or so.
From what I can see, php5-cgi of Debian wheezy is using SetHandler already.  Others would need inspection, too.
Comment 19 Michael Orlitzky gentoo-dev 2015-03-27 15:22:10 UTC
(In reply to Alex Legler from comment #13)
> (In reply to Michael Orlitzky from comment #11)
> > 
> > The GLSAs aren't read by very many people so a regular news item makes more
> > sense.
> 
> [citation needed]

20 years ago, nobody had ever read a GLSA. The burden of proof is on whoever claims that has changed =)

Seriously though, I don't think I ever saw one until I signed up to the gentoo-announce list (for other reasons). There's really no way an ordinary user will discover they exist, much less figure out how to read them. They just don't show up in day-to-day usage of Gentoo, so I would guess that most users don't even know they exist. (Note: I'm not saying they're not valuable, just that they're inconspicuous!)

Of course I can't give you exact numbers of how many people don't do something, but everyone who uses the ::gentoo repo sees the news items. So, there are certainly more people reading the news than reading the GLSAs. Since this change may introduce a security vulnerability, it's better to be safe than sorry and alert as many people as possible.
Comment 20 Sebastian Pipping gentoo-dev 2015-03-28 02:11:30 UTC
I wonder if we should add something like

  # Protect against serving unhandled code/config as plain text
  <FilesMatch "\.(php|php5|phtml|phps)\.">
    # Apache 2.2
    <IfModule !mod_authz_core.c>
      Order deny,allow
      Deny from all
    </IfModule>

    # Apache 2.4
    <IfModule mod_authz_core.c>
      Require all denied
    </IfModule>
  </FilesMatch>

as well.  Trouble is, making it work for both 2.2 and 2.4 seems to
require mod_authz_core or mod_version, both of which can be turned
off by use flags.

https://stackoverflow.com/questions/10707186/detect-apache-version-in-apache-config

What other mean could we use to handle both 2.2 and 2.4?
Could something like

  # Protect against serving unhandled code/config as plain text
  <FilesMatch "\.(php|php5|phtml|phps)\.">
    RewriteEngine on
    RewriteRule .* - [R=404,L]
  </FilesMatch>

work?
Comment 21 Sebastian Pipping gentoo-dev 2015-03-30 00:08:41 UTC
Update:  I am proposing recipe

  <FilesMatch "\.(php|php5|phtml|phps)\.">
    # a) Apache 2.2 / Apache 2.4 + mod_access_compat
    #Order Deny,Allow
    #Deny from all

    # b) Apache 2.4 + mod_authz_core
    #Require all denied

    # c) Apache 2.x + mod_rewrite
    #RewriteEngine on
    #RewriteRule .* - [R=404,L]
  </FilesMatch>

as a base for something to add manually in internal revision 3 of the news item now.

  http://thread.gmane.org/gmane.linux.gentoo.devel/95282

I am grateful about feedback and testing.  Thanks!
Comment 22 Michael Orlitzky gentoo-dev 2015-03-30 01:14:19 UTC
(In reply to Sebastian Pipping from comment #20)
> 
>     # Apache 2.4
>     <IfModule mod_authz_core.c>
>       Require all denied
>     </IfModule>
>   </FilesMatch>
> 
> as well.  Trouble is, making it work for both 2.2 and 2.4 seems to
> require mod_authz_core or mod_version, both of which can be turned
> off by use flags.

That's not a huge problem, since the "Require all denied" line needs mod_authz_core anyway. The protections will just be ignored when apache doesn't have the core auth module. Which is fine, because the user ignored the big warning that it was critical:

  ewarn "Warning: Critical module not installed!"
  ewarn "Modules 'authn_core', 'authz_core' and 'unixd'"
  ewarn "are highly recomended but might not be in the base profile yet."
Comment 23 Sebastian Pipping gentoo-dev 2015-03-30 01:21:29 UTC
I have filed bugs against the official documentation upstream now:

  Security concerns with documentation of AddHandler (and multiple file extensions)
  https://bz.apache.org/bugzilla/show_bug.cgi?id=57777

  Effect of AddType should be documented in more detail
  https://bz.apache.org/bugzilla/show_bug.cgi?id=57778

I also found this existing ticket:

  AddHandler etc. documentation "extension" unintuitive
  https://bz.apache.org/bugzilla/show_bug.cgi?id=57584

If you do have a bz.apache.org account, please vote for it.  I hope that helps making them care.
Comment 24 Sebastian Pipping gentoo-dev 2015-04-05 23:10:37 UTC
+*eselect-php-0.7.1-r4 (05 Apr 2015)
+
+  05 Apr 2015; Sebastian Pipping <sping@gentoo.org>
+  +eselect-php-0.7.1-r4.ebuild, +files/70_mod_php5.conf-apache2-r1:
+  Move from AddHandler to FilesMatch/SetHandler for security (bug #538822)
+

News item pushed to gentoo-news, see

https://gitweb.gentoo.org/proj/gentoo-news.git/tree/2015/2015-04-06-apache-addhandler-addtype
Comment 25 Leho Kraav (:macmaN @lkraav) 2016-03-09 23:16:54 UTC
https://gitweb.gentoo.org/proj/gentoo-news.git/tree/2015/2015-04-06-apache-addhandler-addtype this url gives Repository not found