0

I have a dbconfig.php file which contains the MySQL database name, host, username and password to connect to the database.

// dbconfig.php
$host = '127.0.0.1';
$username = 'root';
$password = '';
$db_name = 'mydb';
$db = new PDO('mysql:host='.$host.';dbname='.$db_name.';charset=utf8', $username , $password);

I have a HTML form which allows you to set from the UI the database credentials, so the strings above come from user input and are saved directly into this file by writing them when the form is submitted.

How can I sanitize the strings so that it can't be exploited? Currently I write the strings like this:

function sanitizeStr($string) {
    $string = str_replace("\\", "\\\\", $string);
    return var_export($string, true);
}

$pattern = "/\\\$host = .*;/";
$newpat = '\$host = ' . sanitizeStr($_POST['db_host']) . ';';
$contents = preg_replace($pattern, $newpat, $contents);

// ... same for $username, $password and $db_name

Are str_replace and var_export enough to stop code from being injected into dbconfig.php?

I can not try a really aggresive sanitization (such as allowing only specific characters) because this would force the user to have a MySQL password which only contans those characters.

How can I make sure that the string from the input will only be used as strings inside the PHP file and not somehow as valid code?

XCS
  • 27,244
  • 26
  • 101
  • 151
  • How is it a duplicate? That talks about sanitizing user input inside PDO quries, this is about sanitizing user input store inside a PHP file? It is not related to SQL. – XCS Mar 19 '20 at 16:03
  • 2
    Why would you want to "sanitize" this input? Against what? Someone who provides the credentials apparently already has the full control. One don't have to break into the house, if he has the key. – Your Common Sense Mar 19 '20 at 16:03
  • @YourCommonSense Because this happens on initial setup of a script. If an attacker happens to be there at the same time, he could execute his own code before you enter your database credentials. Once the data has been entered the form disappears. – XCS Mar 19 '20 at 16:04
  • Plus, my use-case is not really relevant, the question is how to sanitize user input stored inside a PHP file. – XCS Mar 19 '20 at 16:06
  • And how do you distinct a real user from an attacker if no credentials are set yet? – u_mulder Mar 19 '20 at 16:07
  • if your question is how to export a variable then the answer is to use var_export(). I don't really get what's the purpose of str_replace though – Your Common Sense Mar 19 '20 at 16:08
  • 1
    @YourCommonSense I suppose if someone inputs password like `pass'; exec('rm -rf .'); //`. And OP puts it into a file as is - the file will clear the home dir. – u_mulder Mar 19 '20 at 16:09
  • @u_mulder The form is publicly accessible only before the correct database connection information is stored. Once the credentials are entered, the form disappears. If somehow an attacker manages to get to this form between the time you upload the script and by the time you enter your database information, I don't want him to be able to execute any code. If he enters connection information to another database the problem is not as bad, until I find a better solution for the initial setup. – XCS Mar 19 '20 at 16:10
  • @YourCommonSense My question is how to safely write the value of `USER_INPUT` in `$host = 'USER_INPUT';`. @u_mulder mentioned an example of a possible attack. – XCS Mar 19 '20 at 16:14
  • You can think about it "how to prevent PHP injection". – XCS Mar 19 '20 at 16:15
  • Then I already changed the dupe target that should satisfy your needs – Your Common Sense Mar 19 '20 at 16:16
  • Also I suggest storing credentials not in php file. But maybe in some plain txt,ini,json config. – u_mulder Mar 19 '20 at 16:17
  • @YourCommonSense I have also asked in my question if the `var_export` I use is enough, but recently someone suggested that it might not be enough. – XCS Mar 19 '20 at 16:17
  • Then ask someone to prove their point – Your Common Sense Mar 19 '20 at 16:19
  • @u_mulder This dbconfig.php is executed on every request, so I thought that would affect performance. Also, does that actually solve the problem? The values are still used inside the PDO connection string, but I assume in that case PHP will automatically conver them to proper strings. – XCS Mar 19 '20 at 16:19
  • @YourCommonSense That's what I was asking here. Either a reassurance that `var_export` is enough, or an example where it is not and a better sanitization function. I doubt the question was a duplicate of the marked one though. – XCS Mar 19 '20 at 16:21
  • and you already got your answer through a duplicate – Your Common Sense Mar 19 '20 at 16:21
  • @YourCommonSense In answer of the question marked as duplicate there is even a comment: `Bad way, because problem with security. You can included some code with vulnerability.`. I don't understand how the duplicate is relevant just because the answer is the same. It's like I'm asking "what is 4+5" and you link to the question "what is 3*3" just because the answer is the same. – XCS Mar 19 '20 at 16:25
  • Stack Overflow is about answers, not comments. The answer provided covers your issue. – Your Common Sense Mar 19 '20 at 16:29
  • Don't store config in an executable format in the first place and you won't have to worry about problems like this. Dump it into a format like JSON. – Sammitch Mar 19 '20 at 16:30
  • @Sammitch But isn't it a performance issue to read this config on every database query? – XCS Mar 19 '20 at 16:35
  • @RonvanderHeijden Thank you! I see they only use `addcslashes` to sanitize the input. – XCS Mar 19 '20 at 16:39
  • No. Go write an example config file, then write a script that has a loop that reads it 1000 times, and then watch it finish so fast that you question whether it ran at all. – Sammitch Mar 19 '20 at 16:40
  • @Sammitch I know WordPress is not a good example, but as mentioned above, [they use the same way](https://github.com/WordPress/WordPress/blob/master/wp-admin/setup-config.php#L372) of storing their DB credentials. It's used on many sites, so maybe it's not that bad. – XCS Mar 19 '20 at 16:42
  • @Sammitch how about exposing the database credentials to anyone who would request a file? Call it secure? Seriously? – Your Common Sense Mar 19 '20 at 16:44
  • Wordpress stores _their own_ config, and might dynamically write it _once_ at install time when only the _admin_ is touching it. To say nothing of the flaming heap of bad practice that WP is in general. If you want to take user input from internet randos and put it in executable files, that's your funeral. It's not like you see 30+ comment threads on how _good_ an idea is here on StackOverflow. – Sammitch Mar 19 '20 at 16:45
  • @XCS you are about to make a mistake, using some ugly hack instead of a specialized function intended for the purpose. Just a friendly warning. – Your Common Sense Mar 19 '20 at 16:45
  • @YourCommonSense What do you mean? I am already using `var_export`. The `addclashes` was just a remark. – XCS Mar 19 '20 at 16:46
  • @Sammitch In my case it's the same. It is the admin's own config for the platform. It's not intended to be public, but the admin needs the interface for the initial setup. On WordPress you have the same attack possibility, to go and fill in the initial setup form before the admin does. – XCS Mar 19 '20 at 16:46

0 Answers0