Looking for a quick critique

[moltm4785]

Wondering if the below can be made any more efficient and just for those wondering I like writing my own functions I don't like using frameworks. 

 

 

  mb_internal_encoding("UTF-8");
    session_start();
    $ROOT_URL = $_SESSION['ROOT_URL'];
    $ROOT_DIR = $_SESSION['ROOT_DIR'];
    include $ROOT_DIR . 'dbconnection/dbconnection.php';
    //UNSETS THE TABLE NAME AND ID SO IT ISN'T PART OF THE LOOP
    $current_table = $_POST['table_name'];
    $SALT = 'xxxxxxxxxx';
    $date_time = date('Y/m/d H:i');
    $column_names_array = array();
    $column_values_array = array();
    $query = $mysqli -> query("SHOW COLUMNS FROM `$current_table`;");
    while ($fetch = $query -> fetch_array())
    {
        $_POST[$fetch[0]] = (!is_array($_POST[$fetch[0]])) ? $mysqli->real_escape_string($_POST[$fetch[0]]) : $_POST[$fetch[0]];
        if (preg_match('/(created|modified)/', $fetch[0]))
        {
            array_push($column_names_array, $fetch[0]);
            array_push($column_values_array, $date_time);
        }
        elseif (is_array($_POST[$fetch[0]]))
        {
            array_push($column_names_array, $fetch[0]);
            array_push($column_values_array, $mysqli -> real_escape_string(join(',', array_unique($_POST[$fetch[0]]))));
        }
        elseif (!empty($_POST[$fetch[0]]))
        {
            $_POST[$fetch[0]] = (preg_match('/password/i', $fetch[0])) ? sha1($SALT . $_POST[$fetch[0]]) : $_POST[$fetch[0]];
            array_push($column_names_array, $fetch[0]);
            array_push($column_values_array, $_POST[$fetch[0]]);
        }
    }
    $query = "INSERT INTO " . $current_table . "(`" . join("`,`", $column_names_array) . "`) VALUES ('" . join("','", $column_values_array) . "');";
    $query_result = ($mysqli -> query($query)) ? true : false;
    echo $query_result;
 

 

Edited March 22, 2013 by Maq

[requinix]

Efficiency? You have security problems - only a couple but they're big. Deal with those before you start thinking about efficiency.

 

1. Table name isn't validated.

2a. Arrays aren't sanitized.

2b. I could bypass your string sanitizing by renaming textboxes into arrays with one element.

 

As for a critique,

1. Not a fan of storing root paths in the session. Where are they set anyways?

2. The code that supposedly "unsets the table name" doesn't (but it doesn't matter unless there's a "table_name" column in the table).

3. Don't include the semicolons in queries.

4. Not a fan of looking for keywords like "created" or "modified" or "password".

5. array_unique() the POST values? What if the values are numbers like "1,1,2" and I want all three?

6. You may not like frameworks but you should use one. Even if it's your own.

 

And use [code] tags when you post code.

[moltm4785]

 

Efficiency? You have security problems - only a couple but they're big. Deal with those before you start thinking about efficiency.

 

1. Table name isn't validated.

2a. Arrays aren't sanitized.

2b. I could bypass your string sanitizing by renaming textboxes into arrays with one element.

 

As for a critique,

1. Not a fan of storing root paths in the session. Where are they set anyways?

2. The code that supposedly "unsets the table name" doesn't (but it doesn't matter unless there's a "table_name" column in the table).

3. Don't include the semicolons in queries.

4. Not a fan of looking for keywords like "created" or "modified" or "password".

5. array_unique() the POST values? What if the values are numbers like "1,1,2" and I want all three?

6. You may not like frameworks but you should use one. Even if it's your own.

 

And use [code] tags when you post code.

 

I agree thats what I'm building is my own framework. First Thanks for the info but two things 

1. Your right I don't really need to unset the table but for other reasons

2. Can you go into more detail on 2b of first section I had it setup to sanitize everything regardless of what it was but the array would error everything out and so I set it up so that it gets bypassed and then sanitized within the if statement of if(isarray($var))

 

I will go ahead and put those changes you talked about, also I have my full paths starting at the index page itself. Is there a better way to set your paths in one spot and then allow them to propgate throughout the entire site without using sessions?

Edited March 22, 2013 by moltm4785

[moltm4785]

So here is the code with changes, I highlighted what I was talking about with the sanitizing factor. Something that always confused me if $current_table is empty or doesn't exist the page will still run fine and the point or error would still be the same would it not, the query is going to error out. I get its good coding but is that generally a good take on it?

 

 

<?php

    mb_internal_encoding("UTF-8");

    session_start();

    $ROOT_URL = $_SESSION['ROOT_URL'];

    $ROOT_DIR = $_SESSION['ROOT_DIR'];

    include $ROOT_DIR . 'dbconnection/dbconnection.php';

    $current_table = $_POST['table_name'];

    $SALT = 'aB1cD2eF3G';

    $date_time = date('Y/m/d H:i');

    $column_names_array = array();

    $column_values_array = array();

    $query = $mysqli -> query("SHOW COLUMNS FROM `$current_table`;");

    while ($fetch = $query -> fetch_array())

    {

        $_POST[$fetch[0]] = (!is_array($_POST[$fetch[0]])) ? $mysqli->real_escape_string($_POST[$fetch[0]]) : $_POST[$fetch[0]];

        if (preg_match('/(created|modified)/', $fetch[0]))

        {

            array_push($column_names_array, $fetch[0]);

            array_push($column_values_array, $date_time);

        }

        elseif (is_array($_POST[$fetch[0]]))

        {

            array_push($column_names_array, $fetch[0]);

            array_push($column_values_array, $mysqli -> real_escape_string(join(',', $_POST[$fetch[0]])));

        }

        elseif (!empty($_POST[$fetch[0]]))

        {

            $_POST[$fetch[0]] = (preg_match('/password/i', $fetch[0])) ? sha1($SALT . $_POST[$fetch[0]]) : $_POST[$fetch[0]];

            array_push($column_names_array, $fetch[0]);

            array_push($column_values_array, $_POST[$fetch[0]]);

        }

    }

    $query = "INSERT INTO " . $current_table . "(`" . join("`,`", $column_names_array) . "`) VALUES ('" . join("','", $column_values_array) . "')";

    $query_result = ($mysqli -> query($query)) ? true : false;

    echo $query_result;

?>

Edited March 22, 2013 by moltm4785

[haku]

Please use code tags around your code. You can do this with the <> button in the editor.

[moltm4785]

Ok I will do so in the future