Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:120823
Message-ID: <be8990b3-c4d0-d014-6a94-0b75045bdef5@bastelstu.be>
Date: Mon, 17 Jul 2023 14:52:57 +0200
MIME-Version: 1.0
To: internals@lists.php.net
References: <PH8PR10MB63375994E22A3A9321C01C60BF37A@PH8PR10MB6337.namprd10.prod.outlook.com>
 <CA+p1FaamHz=nfa=_bQ+NVg51yoXmnzxfEHpmVNXjaX2pxdCZig@mail.gmail.com>
 <57CF1A83-4861-4AE0-92D4-5724A40A00D0@woofle.net>
 <CA+p1FaYWnEmzP4kywtJ7Mb7L+ypxojHkVEWFR+oqDvn1WX0u-g@mail.gmail.com>
Content-Language: en-US
In-Reply-To: <CA+p1FaYWnEmzP4kywtJ7Mb7L+ypxojHkVEWFR+oqDvn1WX0u-g@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] Security implications of parsing env variables in .ini
From: tim@bastelstu.be (=?UTF-8?Q?Tim_D=c3=bcsterhus?=)

Hi

On 7/14/23 18:03, David Gebler wrote:
>> 2) These expansions should probably be disabled by INI_SCANNER_RAW; that
>> flag already disables certain other types of value interpolation. (Oddly,
>> it doesn't disable expansion of constants either; that might be worth
>> revisiting as well.)
> 
> 
> Environment variable parsing is already disabled by INI_SCANNER_RAW mode,
> isn't it? Personally I don't think the default/normal mode should behave
> differently. If you're passing untrusted input to parse_ini_string, you
> should be sanitizing, white listing or using raw mode anyway really.
> 

Defaults matter. Developers should not need to provide 
INI_SCANNER_PLEASE_DONT_PWN_ME to safely use a function.

Yes, the function is documented to behave "like php.ini's parsing", but 
injecting potentially sensitive environment variables still violates the 
principle of least surprise for me. Nothing about the function's 
behavior or documentation indicates that it might be unsafe to use with 
untrusted input data.

A short term improvement might be adding an explicit yellow warning to 
the documentation page.

Best regards
Tim Düsterhus