php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Request #34949 security flaw in parse_ini_file
Submitted: 2005-10-21 19:40 UTC Modified: 2005-10-21 22:12 UTC
From: dewi at morganalley dot com Assigned:
Status: Not a bug Package: Feature/Change Request
PHP Version: 5.0.5 OS: all
Private report: No CVE-ID: None
View Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
If you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: dewi at morganalley dot com
New email:
PHP Version: OS:

 

 [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>

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2005-10-21 19:55 UTC] tony2001@php.net
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 21:59 UTC] 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 22:12 UTC] tony2001@php.net
>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.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sun Dec 22 05:01:30 2024 UTC