-2
\$\begingroup\$

I am having problems with my code because I am using a system called styleci. Can you please check it? How can I make it better and more secure?

<?php
/*
 * Tecflare Corporation
 * Copyright Tecflare Corporation
 * Provided by the Tecflare Corporation System
 * * Code has been scanned by styleci.io
 */
$host = $_POST['hostname'];
$username = $_POST['username'];
$password = $_POST['password'];
$database = $_POST['database'];
//Verify Connection
$link = mysqli_connect($host, $username, $password, $database);
/* check connection */
if (mysqli_connect_errno()) {
    header('Location: index.php?error');
    die();
}
/* check if server is alive */
if (!mysqli_ping($link)) {
    header('Location: index.php?error');
    die();
}
$conn = new mysqli($hostname, $username, $password, $database);
 $sql = 'CREATE TABLE Administrators (
id INT(6) UNSIGNED AUTO_INCREMENT PRIMARY KEY, 
username VARCHAR(1000),
password VARCHAR(1000)
)';
$conn->query($sql);
 $sql = 'CREATE TABLE Settings (
id INT(6) UNSIGNED AUTO_INCREMENT PRIMARY KEY, 
code VARCHAR(1000),
value VARCHAR(1000)
)';
$conn->query($sql);
 $sql = 'CREATE TABLE Storage (
id INT(6) UNSIGNED AUTO_INCREMENT PRIMARY KEY, 
name VARCHAR(99999),
value TEXT
)';
$conn->query($sql);
 $sql = 'CREATE TABLE Blockedips (
id INT(6) UNSIGNED AUTO_INCREMENT PRIMARY KEY, 
blocked VARCHAR(99999),
value TEXT
)';
$conn->query($sql);
 $sql = 'CREATE TABLE Posts (
id INT(6) UNSIGNED AUTO_INCREMENT PRIMARY KEY, 
name VARCHAR(99999),
author VARCHAR(99999),
value TEXT
)';
$conn->query($sql);
 $sql = 'CREATE TABLE Pages (
id INT(6) UNSIGNED AUTO_INCREMENT PRIMARY KEY, 
name VARCHAR(99999),
value TEXT
)';
$conn->query($sql);
 $sql = 'CREATE TABLE Items (
id INT(6) UNSIGNED AUTO_INCREMENT PRIMARY KEY, 
name VARCHAR(99999),
cost VARCHAR(99999),
description TEXT
)';
$conn->query($sql);
 $sql = 'CREATE TABLE Orders (
id INT(6) UNSIGNED AUTO_INCREMENT PRIMARY KEY, 
email VARCHAR(99999),
Products TEXT
)';
 $sql = 'CREATE TABLE Comments (
id INT(6) UNSIGNED AUTO_INCREMENT PRIMARY KEY, 
name VARCHAR(99999),
about TEXT
)';
$conn->query($sql);
 $sql = 'CREATE TABLE dragdrop (
id INT(6) UNSIGNED AUTO_INCREMENT PRIMARY KEY, 
value TEXT
)';
$conn->query($sql);
 $sql = "CREATE TABLE Plugins (
filename varchar(127) collate utf8_bin default NULL,
        action tinyint(1) default '0',
        PRIMARY KEY  (`filename`)
)";
$conn->query($sql);
$sql = "INSERT INTO Administrators (id, username, password) VALUES ('1', '".$conn->real_escape_string(addslashes($_POST['username_p']))."', '".md5($conn->real_escape_string($_POST['password_p']))."')";
$conn->query($sql);
$sql = "INSERT INTO Settings (id, code, value) VALUES ('1', 'title','Multisite Central')";
$conn->query($sql);
$sql = "INSERT INTO Settings (id, code, value) VALUES ('2', 'maintainanceMode','0')";
$conn->query($sql);
$sql = "INSERT INTO Settings (id, code, value) VALUES ('3', 'welcomemsg','Welcome to Multisite')";
$conn->query($sql);
$conn->query($sql);
$sql = "INSERT INTO Settings (id, code, value) VALUES ('4', 'mail','[email protected]')";
$conn->query($sql);
$sql = "INSERT INTO Settings (id, code, value) VALUES ('5', 'api','')";
$conn->query($sql);
$conn->close();
$data = '<?php
            $hostname="'.$host.'";
            $username="'.$username.'";
            $password="'.$password.'";
            $db_name="'.$database.'";
            ?>';
        $file = '../config.php';
        $handle = fopen($file, 'a');
        if (fwrite($handle, $data) === false) {
            echo 'Can not write to ('.$file.')';
        }
        fclose($handle);
header('Location: index.php?install');
/* close connection */
mysqli_close($link);

The code is also available on GitHub.

\$\endgroup\$
0

1 Answer 1

6
\$\begingroup\$

Security

First of all, you should require that install scripts are deleted after installation, as they are no longer needed.

But since some users will just rename the install directory instead of removing it, making your install scripts secure is a good idea.

Code Execution

An attacker that has access to the install directory and knows some valid database credentials (eg via SQL injection) can gain code execution:

POST install.php
hostname=localhost%00";passthru($_GET['x']);$foo="&username=dbuser&password=dbpassword&database=db

What happens here: %00 cuts off everything after localhost when connecting to the database. However, when writing to a file, %00 does not cut off the rest. An attacker then can simply escape the string context with ", and inject their own commands.

Not only is this a security issue, it's also a usability issue. What if my password is pass"word?

What you should do is escape " in the user input before putting it into the strings.

SQL Injection

you are currently not vulnerable, but what you are doing is really not correct. First of all, you don't need addslashes and real_escape_string.

Secondly, escaping is not the correct approach to SQL injection prevention. Always use prepared statements. They are easier to use, it's harder to mess up, and the resulting code is more readable.

Hashing

It is not ok to use md5 for password hashing, and it hasn't been ok for a very long time. md5 is broken, and it's way too fast. Use bcrypt instead.

Installed Flag

Because you have hardcoded the admin id as '1', it's not possible for an attacker to just add additional admins via this script. But this seems to be more of a coincidence. To avoid problems in the future, you should have a installed flag, to check if everything is already set up. If it is, exit the install script.

Misc

  • You are mixing the functional and object oriented way of using mysqli, which isn't good.
  • file_put_contents is easier to use than fwrite (you don't have to open and close the file yourself).
  • Your code would be better structured if you had functions, such as saveConfig(), createTables(), createSettings(), createAdminAccount(), etc.
\$\endgroup\$
6
  • \$\begingroup\$ Great idea, do you think you can maybe help me with that \$\endgroup\$ Commented Jan 23, 2016 at 18:54
  • \$\begingroup\$ @ThomasWilbur with what? I think all my suggestions are pretty straight forward, they shouldn't be difficult to implement. \$\endgroup\$
    – tim
    Commented Jan 23, 2016 at 19:00
  • \$\begingroup\$ Can you send as a pull request? \$\endgroup\$ Commented Jan 23, 2016 at 19:44
  • 3
    \$\begingroup\$ @ThomasWilbur sorry, I don't have the time for that, but it really shouldn't be that hard to do it yourself. \$\endgroup\$
    – tim
    Commented Jan 23, 2016 at 19:48
  • 3
    \$\begingroup\$ @ThomasWilbur: No one here is obligated to contribute on a 3rd party such as GitHub. You've already received substantial advice from this user. \$\endgroup\$
    – Jamal
    Commented Jan 24, 2016 at 4:14

Not the answer you're looking for? Browse other questions tagged or ask your own question.