0

FYI new to coding, self-taught, go easy..

I have a registration form that creates a new SQL table. The company name they input becomes the table name of a newly created table for each login.

Everything is working great except there is a vulnerability in the variable of the table name.

I am using PHP and MySQL.

I originally had issues if the company name had a space in between 2 words, but I fixed that using str_replace

I was testing a lot and found that if I put a company name in "out", it breaks the code.

I receive this error message:

"Fatal error: Uncaught mysqli_sql_exception: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'out ( id INT UNSIGNED AUTO_INCREMENT PRIMARY KEY, first_name VARCHAR(128) NOT ' at line 1 in C:\Users\reece.grover\OneDrive\XML Website\opregister.php:73 Stack trace: #0 C:\Users\reece.grover\OneDrive\XML Website\opregister.php(73): mysqli->query('CREATE TABLE ou...') #1 {main} thrown in C:\Users\reece.grover\OneDrive\XML Website\opregister.php on line 73"

This seems open to not only errors but also SQL injection.

I learned how to bind_param but that still stops the potential code-breaking of the CREATE TABLE function.

Here is my server-side PHP code, the form is just a simple HTML form with Bootstrap.

    //remove spaces
    $company = str_replace(' ', '', $_POST["company"]);
    //hash Password
    $password_hash = password_hash($_POST["password"], PASSWORD_DEFAULT);
    
    // Create new Operator
    $mysql = require __DIR__ . "/database.php";
    
    $sql = "INSERT INTO operators (first_name, last_name, email, password_hash, company) 
            VALUES (?, ?, ?, ?, ?)";
    
    $stmt = $mysql->stmt_init();
    
    
    if ( ! $stmt ->prepare($sql)){
        die("SQL error: " . $mysql->error);
    }
    
    $stmt->bind_param("sssss",
                        $_POST["first_name"],
                        $_POST["last_name"],
                        $_POST["email"],
                        $password_hash,
                        $company);
                         
    if ( ! $stmt->execute()) {
        die($mysqli->error . " " . $mysqli->errno);
    }
    
    
    
    //Create the new company table
    
    $sql = "CREATE TABLE $company (
    id INT UNSIGNED AUTO_INCREMENT PRIMARY KEY,
    first_name VARCHAR(128) NOT NULL,
    last_name VARCHAR(128) NOT NULL,
    email VARCHAR(255) NOT NULL UNIQUE,
    company VARCHAR(128),
    credit VARCHAR(60),
    phone VARCHAR(60))";
    
    if ( ! $mysql->query($sql)) {
        die("Create Table Failed : " . $mysql->error);
    }
    
    $sql = "INSERT INTO $company (first_name, last_name, email, company, credit, phone) 
            VALUES (?, ?, ?, ?, ?, ?)";
    
    $stmt = $mysql->stmt_init();
    
    
    
    if ( ! $stmt ->prepare($sql)){
    die("SQL error: " . $mysql->error);
    }
    
    $stmt->bind_param("ssssss",
                        $_POST["first_name"],
                        $_POST["last_name"],
                        $_POST["email"],
                        $company,
                        $_POST["credit"],
                        $_POST["phone"]);
                         
    if ($stmt->execute()) {
    
        header("Location: signup_success.php");
        exit;
    
  • 4
    Bad database design. Create a table `companies` and place all of your companies in it – brombeer May 30 '23 at 07:32
  • 1
    "it breaks the code" - what does that mean? Are you facing a specific error? – Nico Haase May 30 '23 at 07:34
  • 3
    A registration form should NEVER create a new table in the first place. – Your Common Sense May 30 '23 at 07:35
  • note that if you care for security, you should never do anything like `die($mysqli->error` as it reveals A LOT about your system for a potential attacker. Please read here: https://phpdelusions.net/basic_principles_of_web_programming#error_reporting – Your Common Sense May 30 '23 at 07:41
  • 2
    Regarding your specific error, see https://stackoverflow.com/questions/23446377/syntax-error-due-to-using-a-reserved-word-as-a-table-or-column-name-in-mysql and https://dev.mysql.com/doc/refman/8.0/en/keywords.html. But as discussed I think you have more issues to rethink. – Don't Panic May 30 '23 at 07:48
  • @NicoHaase I have edited my post to show the error when entering out as company name – Reece Grover May 30 '23 at 07:51
  • You can `SELECT * FROM users WHERE company_id=12` f.e. to only get users that work for company with an id of 12. And also remove those users – brombeer May 30 '23 at 07:51
  • 1
    Remember what you said about being "new to coding"? WHY NOT to listen what people with DECADES of experience telling you? And change your whimsic "I want" to "thank you for the heads up"? – Your Common Sense May 30 '23 at 07:52
  • That's ABC of database design. Each user is able to see their own companies linked to their id. – Your Common Sense May 30 '23 at 07:54
  • @YourCommonSense fair call man, Thanks brombeer. – Reece Grover May 30 '23 at 07:55
  • 2
    Each user is only able to see what you show them. Users don't have access to your tables. A simple SELECT * FROM company WHERE user_id=1 will show a user their companies. This IS how databases work – Your Common Sense May 30 '23 at 07:59
  • @brombeer makes perfect sense. I think I was just committed to the hours I spent so I did the first design. You are all legends, thank you all for your ultimate wisdom – Reece Grover May 30 '23 at 08:05

1 Answers1

1

You should have a pre-defined companies table instead of individual company_name tables. You should also absolutely not be creating tables based on user input.

Create the companies table beforehand:

CREATE TABLE companies (
  id INT PRIMARY KEY AUTO_INCREMENT,
  company_name VARCHAR(255),
  email VARCHAR(255),
  phone VARCHAR(20)
);

Then you can INSERT the data into this table:

$stmt = $conn->prepare("INSERT INTO companies (company_name, email, phone) VALUES (?, ?, ?)");
//Example company data
$companyName = "Example Company";
$email = "info@example.com";
$phone = "123-456-7890";
$stmt->bind_param("sss", $companyName, $email, $phone);
$stmt->execute();

Each company is inserted into this table. They are distinguished by the id column.

You can also add a foreign key constraint on the operators table.

ALTER TABLE operators
ADD COLUMN company_id INT,
ADD CONSTRAINT key_constraint_company_id FOREIGN KEY (company_id) REFERENCES company(id);

If you want the company data deleted when the operator is deleted, you can do a DELETE CASCADE:

ALTER TABLE operators
ADD COLUMN company_id INT,
ADD CONSTRAINT key_constraint_company_id FOREIGN KEY (company_id) REFERENCES company(id) ON DELETE CASCADE;
GenesisBits
  • 364
  • 2
  • 23