Harden open_basedir restrictions in various extensions (or even deprecate open_basedir)
まだ誰も着手していません。
- 主要言語
- C
- スター
- 40.4k
- フォーク
- 8.1k
- 平均マージ
- 2日 13時間
- マージ済み PR(30日)
- 96
説明
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 asgdbm,qdbm, orlmdbopen 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.mofiles from that location.ext/openssl: SSL context options such ascafile,capath, anddh_parammay be passed to OpenSSL APIs such asSSL_CTX_load_verify_locations()orBIO_new_file().ext/gd: FreeType font loading can pass the resolved font path toFT_New_Face()after locating it withaccess().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.
コントリビューションガイド
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
調査の方向性
まず main/fopen_wrappers.h を読み、ネイティブなファイルアクセスについて ext/dba、ext/gettext、ext/openssl、ext/gd、ext/standard にある対象の呼び出し箇所を監査します。提供されている DBA、gettext、stream_resolve_include_path の例を再現用として使用します。影響を受ける関数が一貫して open_basedir の制限に従い、示されたパスに対するリグレッションテストのカバレッジが確保されていれば完了です。
索引モデルが issue の本文から書いたものです。
評価
- 技術スタック
- c, php
- 領域
- security
- issue の種類
- バグ
- 難易度
- 5/5
- 見積もり時間
- 1週間以上
- 活発さ
- 静か
- 明瞭さ
- 説明が足りない
- 初心者へのやさしさ
- 35/100