rfc:script_only_include

PHP RFC: Introduce script only include/require

Introduction

include()/require() is source of file/script inclusion for a long time. Script inclusion is extremely severe security breach and PHP is too weak against script inclusion attacks with respect to other languages.

  • PHP (many and these are _only_ few of them in the wild)
  • PERL (0 result)
  • Rails (0 result)
  • Python (0 result)
  • JSP (1 result - This is famous)

The Root of This Difference

The root cause of the difference is how scripts are loaded.

Python

Python loads script like

import foo

where foo is foo.py.

Ruby

Ruby loads script like

require 'foo'

where foo is foo.rb.

PHP

PHP loads script like

require 'foo.php'

where foo.php is the script loaded. i.e. Raw/full filename is specified as script to be loaded.

Other Languages

Other languages provides loading script without file extension. This prevents “script inclusion” attacks effectively. Above number of vulnerabilities is the proof.

Other Aspects

There are other reasons that PHP is weaker than other languages.

  • PHP is embedded language by default. Therefore, many users use include/require for loading templates. i.e. Users casually use script loading include/require with variables.
  • Embedded mode cannot be disabled. Even when only script is needed, users have to use include/require. Users casually use script loading include/require with variables as custom due to PHP's embed nature.
  • In other languages, even if there is vulnerability in script loading code, they cannot read any files unlike PHP. PHP allows to read any files like require('/etc/passwd').

One of the main reason why people start using scripting languages is security. However, PHP is extremely weaker against script/file inclusion compare to other language above reasons.

Even though this RFC does not change PHP's embed anything feature, this RFC changes security impacts of above aspects.

Solution

Solution for this issue is simple. Introduce script only include/require and modify move_uploaded_file() function not to move PHP script embedded files. With this RFC, PHP becomes as secure as other languages against script/file inclusion attacks via script loading features.

Discussions

Do not see how this RFC prevent script inclusion attacks

  • include*()/require*() refuse to compile/execute file extensions other than “.php .phar” by default.
  • move_uploaded_file() refuse to move PHP script. “.php .phar” is refused by default.

With this RFC, include*()/require*() only executes files have “.php” or “.phar” extension and move_uploaded_file() refuse to move uploaded files that can be executed as PHP script. Therefore, even most obvious mistake like 'include $_GET[“var”];' will not work anymore. i.e. It cannot read files like “include '/etc/passwd';” nor execute script like “include '/path/to/upload/evil_image.jpg';”.

This RFC gives false sense of security like magic_quotes/etc

This is not true. This RFC does not change any inputs, does not have any cons except slight BC issue. This RFC protects users from security disaster. If you insist this RFC gives false sense of security, give reasons why. When user write code like 'include $_GET[“var”];' or 'move_uploaded_file($src, “/path/to/upload/mygreatscript.php”);', PHP dies with E_ERROR/warns with E_WARNING. This is not false sense of security, but actual security warning that users must take care of.

Why not check embedded script in parser?

Checking embedded PHP script by parser/etc never works. For example, text file with PHP script example is valid file. If we forbid embedded PHP script, we forbid valid file also.

This RFC breaks too many applications

This can be fixed by one liner.

  ini_set('zend.script_extensions', '.php .phtml .inc .module .etc');
  or
  ini_set('zend.script_extensions', ''); // The same as now. Disable mitigations. Not recommended. 

ini_set() does not emit any errors. Therefore, this one liner works for all PHP versions.

My app has special entry point like .gif/.html

If you are running PHP as web server module, you can use any file extensions as PHP scripts. See “Implementation Details”.

OS protection is not perfect protection

OS protection works well for system files. However, any applications support file uploads can be attacked by inclusion attacks. e.g. require('../../upload/evil_file') can be done with bad PHP code.

open_basedir is not helpful

open_basedir restrict file reads, but it's not helpful. Attackers use uploaded files to exploit PHP applications. These uploaded files are images, document, etc. Session data file is common target for attackers also.

Uploaded Files

Even though move_upload_file() is obsolete function, users are supposed to use move_uploaded_file() function to move uploaded files to destination directory. move_uploaded_files() is modified to prevent/refuse to move “PHP scripts” without user setting.

Note: Currently, there is no real difference between rename() and move_uploaded_file() as long as users use tmp filename in $_FILES for source path.

Do not like new INI

It is our responsibility that provides reasonable security measures against fatal security breach if it is feasible. This RFC provides systematic security measures against script/file inclusion vulnerability. New INI cannot be good reason to refuse this RFC that closes major security hole in PHP programs.

Users must be careful in first place

I totally agree with this opinion. Users must be careful. However, many PHP developers/users suffer “script inclusion vulnerability” unlike other languages. “Script inclusion vulnerability” is fatal and unacceptable risk. Therefore, we are better to have mitigations.

Proposal

Introduce php script extension configuration:

Web SAPI

zend.script_extensions = ".php .phar" ; Allow only *.php and *.phar. User may add/change this INI setting.

CLI SAPI

zend.script_extensions = NULL ; Allow any file for CLI by default. Where NULL is empty or ''

PHP checks file extension if it is allowed to load as PHP script when loading scripts by require/require_once/include/include_once.

If file does not have expected extension, require/require_once/include/include_once emits E_ERROR.

Introduce $allow_script flag to move_uploaded_file():

  bool move_uploaded_file(string $filename , string $destination [, $allow_script=FALSE ] )

With current PHP, move_uploaded_file() has no real protection. Add new flag and make it useful again. It returns FALSE and raises E_WARNING for PHP script when $allow_script=FALSE.

Old proposals

  • Detect “<?php” at the top of file and disallow “?>”: This still allows to load PHP script like 'require “image.png”;'/etc.
  • Introduce script()/script_once(): The same issue as above.
  • Introduce embed_mode INI: The same issue as above.

Backward Incompatible Changes

  • If users have different filename extensions other than de facto standard, they have to modify “zend.script_extensions” INI or they can completely disable mitigations by 'ini_set(“zend.script_extensions”, “”)'.
  • move_uploaded_file() cannot move PHP scripts unless $allow_script=TRUE. Note: ini_set(“zend.script_extensions”, “”) allows to move any files.

Proposed PHP Version(s)

PHP 7

RFC Impact

To SAPIs

Web SAPI

  • New “zend.script_extensions” INI - “.php .phar” are allowed as PHP script. See implementation details also.

CLI SAPI

  • New “zend.script_extensions” INI - No BC impact since it allows any file as PHP script by default. Users may set zend.script_extensions to use mitigations.

To Existing Extensions

Extensions that have custom script loader with custom extension. “zend.script_extensions” modification may be needed to work.

To Opcache

None

New Constants

None

php.ini Defaults

zend.script_extensions: INI_ALL. Script may modify this at anytime.

  • hardcoded default values
    • Web SAPI: “.php .phar”
    • CLI SAPI: NULL
  • php.ini-development values
    • NULL (Empty. Use the default)
  • php.ini-production values
    • NULL (Empty. Use the default)

Open Issues

  • Error type for include/require - Use E_ERROR
  • Vote type - Use 2/3

Unaffected PHP Functionality

All of require/require_once/include/include_once are affected by this.

Future Scope

  • Allow script without PHP tag.

Proposed Voting Choices

State whether this project requires a 2/3 or 50%+1 majority (see voting)

Requires 2/3 “yes” to pass. Vote ends 2015/3/12.

Introduce script inclusion protection?
Real name Yes No
aharvey (aharvey)  
ajf (ajf)  
bwoebi (bwoebi)  
derick (derick)  
dm (dm)  
frozenfire (frozenfire)  
galvao (galvao)  
guilhermeblanco (guilhermeblanco)  
ircmaxell (ircmaxell)  
jedibc (jedibc)  
kinncj (kinncj)  
leigh (leigh)  
mbeccati (mbeccati)  
mfischer (mfischer)  
mike (mike)  
mrook (mrook)  
nikic (nikic)  
olemarkus (olemarkus)  
pajoye (pajoye)  
pollita (pollita)  
rasmus (rasmus)  
rdlowrey (rdlowrey)  
santiagolizardo (santiagolizardo)  
seld (seld)  
stas (stas)  
thijs (thijs)  
till (till)  
yohgaki (yohgaki)  
Final result: 4 24
This poll has been closed.

Patches and Tests

Implementation Details

The patch does not protect “direct access”. i.e. It allows to execute PHP scripts specified by web server configurations when PHP is a module of web server. e.g.

<FilesMatch \.png$> SetHandler application/x-http-php </FilesMatch>

allows “.png” files to be executed as PHP script regardless of “zend.script_extensions”. When PHP script files are opened by Web server, PHP will not check file extensions. The patch checks extensions only when script files are loaded by PHP/Zend.

The patch works only under non-ZTS build currently. Under ZTS build, move_uploaded_file() does not work. This issue is addressed before merge.

Implementation

  1. the version(s) it was merged to
  2. a link to the git commit(s)
  3. a link to the PHP manual entry for the feature

References

Rejected Features

Keep this updated with features that were discussed on the mail lists.

ChangeLog

  • Added more “Discussions” - 2015/02/26
  • Added “Implementation Details” section - 2015/02/24
  • Added patch. Adjusted RFC for code optimization. - 2015/02/22
  • Changed “php_scripts” to “zend.script_extensions”. - 2015/02/22
  • Added error type/INI type. Fixed typo. - 2015/02/22
rfc/script_only_include.txt · Last modified: 2018/03/01 23:19 by carusogabriel