|
|
php.net | support | documentation | report a bug | advanced search | search howto | statistics | random bug | login |
[2005-10-21 19:40 UTC] 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>
PatchesPull RequestsHistoryAllCommentsChangesGit/SVN commits
|
|||||||||||||||||||||||||||
Copyright © 2001-2024 The PHP GroupAll rights reserved. |
Last updated: Fri Apr 19 21:01:30 2024 UTC |
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.