months into php mysql and security is a headache

[pheidole]

I have copious amounts of books on php that i have been reading for the last 10 months.. My favorite one so far has been latest version of

PHP & MySQL: Novice to Ninja..

there are a cple other books i have read and continue to read that are extremely helpful. Unfortunately i purchased a cple books that teach you outdated programming: functions like mysql_connect and so on. Anyhow i found that coding in php is rather fun and the basics of of how php and mysql works it not extremely challenging to figure out.  Its unlimited to what you can do it seems. I find the hairy part to be SECURITY. I find it challenging to write code that is secure.. I have written a few basic scripts and posted the code in a few different spots asking for help when i get stuck. The code i write does what is suppose to do most of the time, if not i figure it out with help, but guess what ? ill get a comment from an experienced programmer that tells me my code is prone to sql injection and so on. Its just a headache and makes learning php and mysql rather tedious.you spend hours coding just to find out there are 5 possible exploits in 250 lines of code. Fortunately i find it fun and addicting so i do not plan to stop anytime soon. PS thanks for the helpful group

Edited November 7, 2018 by pheidole

[mac_gyver]

if your comment/question is just about database security, that's easy to do. if an sql query has external/unknown data values being supplied to it, use a prepared query. if are no pieces of data being supplied to the sql query or there are but they are 100% known to be safe (were produced by your server-side code), use a non-prepared, regular query. you would also use a prepared query for the few cases where you need to execute the same sql query statement multiple times with different sets of input data.

the key to making this easy is to use the simplest and least amount of implementation code, so that you are spending most of your time working on the logic that accomplishes a task, rather than on the implementation details.

the only php code i/we have seen of your's on this forum is this - https://forums.phpfreaks.com/topic/307852-not-pulling-data/

that particular code can be simplified by -

1) using exceptions to handle database statement (connection, query, prepare, execute) errors, and in most cases let php catch the exception where it will use its error_reporting, display_errors, and log_errors settings to control what happens with the actual error information. you would display all errors when learning, developing, and debugging code/queries. when on a live/public server, you would log all errors. doing this only takes one statement to enable exceptions for the database extension you are using. you will then be able to remove all the conditional logic you have now that's trying to provide error handling for the database statements (in at least one case in that code, you are not reporting the error, so, the existing logic isn't always helping.)

2) use the much simpler php PDO extension. this will reduce the amount of code (you don't need to explicitly bind input or result data or use the get_result statement) and lets you treat the result from all queries the same. an added advantage to using the PDO extension is that the same php statements can be used regardless of the database type (you will still need to make any necessary changes to the actual sql query syntax for different database types.) with the mysqli extension, those statements are specific to the mysql (and cloned) database type. if you want or need to switch to a different database type - mssql, postgre, sqllite, ... , you will need to learn another set of php statements specific to that database type. 

3) separate the different concerns in your code. the database specific code, that knows how to execute a query and fetch the data, is a different concern from the presentation code, that knows how to produce the output from the data. the way to accomplish this separation is put the database specific code above the start of the html document (or ajax response code), fetch the data from queries into appropriately named php variables, then use those variables in the correct place in the html document (or ajax response code.) this will get the clutter of the database code out of the html document, allow you to easily test if your code is producing the correct data, and treat the html document as a 'template' where you only have simple php code in the html document to use the data being supplied to it. also, by fetching all the data from any query, you eliminate synchronization errors, so there is no longer a need to close prepared statements.

4) php will automatically close database connections when the script ends, so unless you have a good reason to do so, don't close them in your code.

a slightly more advanced thing you can do that will make your implementation code more general purpose, is to extend the database extension you are using with your own general purpose prepared/non-prepared query method,, that accepts the sql query statement and a second optional array parameter of input data. the method code will test for the optional input data and run the code needed for a prepared query if there is input data, and run a non-prepared, regular query if there is not.

the code from that thread, making use of items 1-4, would (untested) simply be -

<?php

// since this code is apparently only providing a response to an ajax request, only execute it if the expected input is set

if(isset($_GET['term']) && $_GET['term'] != '')
{
	// make the pdo connection - you would typically put common code like this into an external .php file, then require it when needed
	$DB_HOST = 'localhost';
	$DB_USER = 'root'; // you should not use the root account in your application. make a specific user with just the permissions that your application needs
	$DB_PASS = '';
	$DB_NAME = 'st';
	$DB_ENCODING = 'utf8'; // db character encoding - set to match your database table character set (utf8 is a common/best choice)

	$pdo = new pdo("mysql:host=$DB_HOST;dbname=$DB_NAME;charset=$DB_ENCODING",$DB_USER,$DB_PASS);
	$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); // set the error mode to exceptions
	$pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES,false); // run real prepared queries
	$pdo->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE,PDO::FETCH_ASSOC); // set default fetch mode to assoc

	
	// database specific code
	$search_result = []; // define/initialize to an empty array
	$sql = "SELECT id, state FROM loc WHERE state LIKE ?";
	$stmt = $pdo->prepare($sql);
	$stmt->execute([$_GET["term"] . '%']);
	$search_result = $stmt->fetchAll();


	// presentation code
	if(empty($search_result))
	{
		// no data to display - it's usually clearer if you handle error/negative results first
		echo "<p>0</p>"; // this what your original code output when there were no matching rows
	}
	else
	{
		// data to display
		foreach($search_result as $row)
		{
			echo "<p>" . $row["state"] . "</p>";
		}
	}
}

 

 

 

 

Edited November 7, 2018 by mac_gyver

[kicken]

When it comes to query parameters, I like to create a parameterize function which generates a placeholder name and adds the value + name to an array then returns the new parameter name.  This allows concatenation like one might be used to but uses parameters.

function Parameterize($value, &$binds){
    static $counter = 0;

    if (!is_array($binds)){
        $binds = [];
    }

    if (is_array($value)){
        if (count($value) == 0){
            return 'null';
        } else {
            $allParams = [];
            foreach ($value as $v){
                $allParams[] = Parameterize($v, $binds);
            }

            return implode(',', $allParams);
        }
    } else {
        if (is_bool($value)){
            $value = (int)$value;
        } else if ($value instanceof \Datetime){
            $value = $value->format('Y-m-d H:i:s');
        }

        $param = ':param' . (++$counter);
        $binds[$param] = $value;

        return $param;
    }
}

With that, the query above would look like:

// database specific code
$sql = "SELECT id, state FROM loc WHERE state LIKE " . Parameterize($_GET['term'] . '%', $binds);
$stmt = $pdo->prepare($sql);
$stmt->execute($binds);

If you want to do multiple queries in a row you need to clear out the variable holding the parameters ($binds above) before each query or you'll get an error due to mismatching parameters.  I tend to isolate my queries into their own function which is another way to avoid this problem as each function has it's own scope.