php / php/php-src

Harden open_basedir restrictions in various extensions (or even deprecate open_basedir)

Open
#21,971 10 comments 3 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Feature Status: Needs Triage
Dominant language
C
Stars
40.4k
Forks
8.1k
Avg merge
2d 13h
Merged PRs (30d)
96

Description

Description

Now, several extensions in the php codebase has functions with the ability to read/write outside the open_basedir restrictions.

That is because, the open_basedir restrictions only works on PHP streams, and some of our extensions read/write without it, and therefore bypassing the open_basedir check.

Examples found while auditing similar open_basedir behavior:

  • ext/dba: non-stream DBA handlers such as gdbm, qdbm, or lmdb open paths through their backend libraries, e.g. gdbm_open(), dpopen(), mdb_env_open().
  • ext/gettext: bindtextdomain() resolves a directory and then passes it to libc/gettext, which later loads .mo files from that location.
  • ext/openssl: SSL context options such as cafile, capath, and dh_param may be passed to OpenSSL APIs such as SSL_CTX_load_verify_locations() or BIO_new_file().
  • ext/gd: FreeType font loading can pass the resolved font path to FT_New_Face() after locating it with access().
  • ext/standard: stream_resolve_include_path

IMO, all of them are supposed to be fixed, that they should align with the expected open_basedir behavior. I know that the above only work with conditions (e.g. know dba keys for dpopen, or being a .mo file for bindtextdomain) but all of them should only works under the restrictions of open_basedir due to serious safety concerns. Which is simply by adding

#include "main/fopen_wrappers.h"

if (php_check_open_basedir(path)) {
    RETURN_FALSE;
}

/* then call native/library open */

I don't think this requires a RFC so I would like to directly open this issue to discuss about this. cc @iluuu1994 . Thanks!

Below are some example payloads:

<?php
ini_set('open_basedir', __DIR__ . '/allowed');

$db = dba_open('/tmp/outside.gdbm', 'r', 'gdbm');
var_dump(dba_fetch('secret', $db));

and

<?php
ini_set('open_basedir', __DIR__ . '/allowed');

bindtextdomain('leak', '/tmp/outside-locale');
textdomain('leak');

echo gettext('secret_key'), "\n";

and (this can only check if a file exists, but without any restrictions)

<?php
ini_set('open_basedir', __DIR__ . '/allowed');
ini_set('include_path', '/etc');
var_dump(stream_resolve_include_path('passwd'));
var_dump(@file_get_contents('passwd', use_include_path: true));

I don't sure if we should treat this as a security issue :) This research is done with @q1uf3ng

TL;DR some of the functions use zend_resolve_path for file IO, the API doesn't check if it fits in the open_basedir restrictions.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading main/fopen_wrappers.h and auditing the call sites named in ext/dba, ext/gettext, ext/openssl, ext/gd, and ext/standard for native file access. Use the provided DBA, gettext, and stream_resolve_include_path examples as reproductions. Done means the affected functions consistently honor open_basedir restrictions and have regression coverage for the demonstrated paths.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, php
Domain
security
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.