ID: 34949
Updated by: [EMAIL PROTECTED]
Reported By: dewi at morganalley dot com
-Status: Open
+Status: Bogus
Bug Type: Feature/Change Request
Operating System: all
PHP Version: 5.0.5
New Comment:
>get_defined_constants() is code.
>"echo DB_PASS;" is code.
So is parse_ini_file().
>Allowing a user to provide a data file to a program is not
>"allowing users to execute code". It is required
>functionality of many programs.
Don't be surprised when somebody do `rm -rf /` because you didn't check
the input. That *your* duty to do it.
Or do you trust all the data you get from untrusted sources?
Probably not?
I'm sure you don't.
No bug or security flaw here, that's programmer's problem.
Previous Comments:
------------------------------------------------------------------------
[2005-10-21 21:59:02] dewi at morganalley dot com
get_defined_constants() is code.
"echo DB_PASS;" is code.
Data in an ini file is data file, not code.
It can come from untrusted sources, such as when a client program
passes data to the server in ini file format, when a user uploads an
ini file to a syntax checking script, etc.
It is clearly data, and should be treated as such.
Would you have mysql_fetch_assoc() expand constants in the
user-provided values it reads from the DB? Probably not.
Would you have readline() expand constants in the values it reads from
the user? Probably not.
Would you have $_QUERY expand constants in the values it reads from the
HTML form? Probably not.
These, like ini files, are data sources, and you would not trust them.
Why do you trust the data in ini files in this unique and dangerous
way?
Allowing a user to provide a data file to a program is not "allowing
users to execute code". It is required functionality of many programs.
Given that this function exists, the most likely data file format that
programmers will use is the ini format, because it's made easy by this
function.
If a function which parses user data causes internal program data to be
exposed, with no way to disable it, then I feel that is something that
programmers may be caught out by, and should at least be able to turn
off.
For the moment, the only way to securely parse an externally provided
ini file with this function is with a pre-parser like:
$file =
preg_replace('/^(\s*\w+\s*=\s*)((?:(?!\s;)[^"\r\n])*?)(\s*(?:\s;.*)?)$/mx',
'\1"\2"\3', $file);
Most programmers will not think to do this, which is why I submitted
this as a security gotcha.
------------------------------------------------------------------------
[2005-10-21 19:55:03] [EMAIL PROTECTED]
So you consider get_defined_constants() and just "echo DB_PASS;" as
dangerous too, right?
Don't you think that allowing to execute any code to users is the
source of the problem?
------------------------------------------------------------------------
[2005-10-21 19:40:52] dewi at morganalley dot com
Description:
------------
A PHP parse_ini_file() security gotcha.
The auto-expansion of unquoted string values to constants is a problem,
both for strings like 'none', 'true', 'false', 'yes', 'no', 'on', 'off'
(where it can cause unexpected behaviour), and most importantly, for
named constants.
This can cause security issues, in situations where untrusted users are
allowed to create ini files.
eg: you allow untrusted users to create ini files, with values for
name, password, and description. Your script holds its own database
password in a constant "DB_PASS". If the user sets their description to
the unquoted value DB_PASS, your application will display its password
where normally it would display their description.
You can avoid this when creating ini files automatically, if you ALWAYS
quote your string values, and ALWAYS check that numerics are truly
numeric.
But you can't avoid it with user-provided ini files without pre-parsing
them beforehand looking for unquoted string values, or rolling your own
version of this function.
For this function to remain secure with user-provided ini files, I
request an extra, optional boolean parameter, to disable expansion of
constants.
Reproduce code:
---------------
Ini file, "user_provided.ini":
desc = DB_PASS
PHP file:
<?php
define('DB_PASS', 'ungue55able_pa55word');
$user = parse_ini_file("user_provided.ini");
# Reasonable steps to ensure user-provided data is "safe" to display.
if (empty($user['desc'])) { die("Bad ini file."); }
$safe_desc = htmlspecialchars($user['desc']);
# Despite that, we print out insecure info if we use the ini file
above.
echo "<p>Your description is: $safe_desc</p>\n";
?>
Expected result:
----------------
Despite reasonable checking to ensure that there is nothing "naughty"
in the provided ini file, the user's description will still contain
supposedly secure data: the script's database password.
Actual result:
--------------
<p>Your description is: ungue55able_pa55word</p>
------------------------------------------------------------------------
--
Edit this bug report at http://bugs.php.net/?id=34949&edit=1
