Jump to content

Is this PDO database connection/ insertion secure from injection?


ageattack
Go to solution Solved by Jacques1,

Recommended Posts

<?php

if(isset($_POST['submit'])){

$uname = $_POST['username'];
$pword = $_POST['password'];

/*** mysql hostname ***/
$hostname = 'localhost';

/*** mysql username ***/
$username = 'root';

/*** mysql password ***/
$password = 'anty90';

try {
    $link = new PDO("mysql:host=$hostname;dbname=gambling", $username, $password);
    /*** echo a message saying we have connected ***/
    echo 'Connected to database<br />';

    /*** INSERT data ***/
    $stmt = $link->prepare("INSERT INTO gamb(username, password) VALUES (?, ?)");
	
	try{
	$stmt->execute(array("$uname", "$pword"));
	} catch(PDOException $e){
	
		echo "Exception caught: $e";
	
	}

    /*** echo the number of affected rows ***/
    //echo $count;

    /*** close the database connection ***/
    $link = null;
    }
catch(PDOException $e)
    {
    echo $e->getMessage();
    }
  
}
?>

<html>

<form action='home.php' method='post'>

<input type="text" name="username" >
<input type="password" name="password" >
<input type="submit" name="submit" value="submit">

</form>

</html>

I'm new to databse programming so I was just wondering if this was vulnerable to sql injection or not.

Link to comment
Share on other sites

It's good that you are using prepared statements.  There are 2 things I would change though:

1) Store your db credentials in a separate file, and then include it on any page you use the db on. This way if the info changes, you just update one file instead of all files that you use db in. You might also just include the $link statement there too.

2) Don't output db errors to the screen, unless this is only for development server. This exposes info about your db and structure that might be useful to a malicious user. It also does the user absolutely no good to know about your db errors. Log them to a file or send an email with the error or something, and just let the user know "an error occurred" or something more generic.

Link to comment
Share on other sites

This is not a prepared statement!

 

By default, PDO just takes the parameters, escapes them and literally inserts them into the query template. So this is really just a kind of auto-escaping. You might as well escape the values yourself.

 

If you want actual prepared statements, you need to set the PDO::ATTR_EMULATE_PREPARES option to false:

$link = new PDO("mysql:host=$hostname;dbname=gambling", $username, $password, array(
    PDO::ATTR_EMULATE_PREPARES => false    // important! turn off fake prepared statements
));

And, yeah, do get rid of all those useless (and harmful) try statements. I have no idea what you're trying to achieve with them. Printing the error messages on the screen is already the default behaviour on a development system. There's absolutely no point in writing that down. Even worse, now the error messages are always printed on the screen, even if you publish the website on the internet.

Edited by Jacques1
Link to comment
Share on other sites

In addition to these as has been mentioned above you should always be closing the statement as soon as it's not longer needed. If two applications are running and one is trying to modify something like update or delete table / column records or so, while the second one is accessing those objects as far as I know you will receive an mysql error.

Link to comment
Share on other sites

There's no such problem. UPDATE and DELETE queries lock the row or table, and if other processes need to access the same data, they wait until the lock is released.

 

Also note that it's not possible to explicitly close a prepared statement in the PDO extension. The best you can do is kill the object in order to trigger the destructor.

Link to comment
Share on other sites

By closing the statement I meant to kill the statement (bad habit from linux machines), but not the entire object as in the example above. If you want to execute the statement more than once along with others, we can use closeCursor(). This is your example, so I wasn't able to run this script without PDOStatement::closeCursor() and I am interested on how you did :)

<?php

$database_options = array(
    PDO::ATTR_EMULATE_PREPARES => false
    , PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
    , PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC
);
$database = new PDO('mysql:host=localhost;dbname=test;charset=utf8', 'root', '', $database_options);



show_prepared_statements();

// *One* prepared statement
$stmt = $database->prepare('SELECT 1 = :x');
// ... getting executed 10 times
for ($i = 1; $i <= 10; $i++)
{
    $stmt->execute(array('x' => $i));
}

echo '<br>';
show_prepared_statements();



function show_prepared_statements()
{
    global $database;

    $statements = $database->query("SHOW SESSION STATUS LIKE 'Com_stmt_%'");
    foreach ($statements as $statement)
    {
        echo $statement['Variable_name'] . ': ' . $statement['Value'] . '<br>';
    }
}
Edited by jazzman1
Link to comment
Share on other sites

You're getting an error about an unbuffered query, right? That's an issue of an unfetched result set, not the statement itself.

 

By default, PHP automatically fetches the entire result set into memory. However, if you (unwillingly) disable this, make a query, don't fetch all rows an then make another query (like the code above), you're getting an error.

 

Sure, you can use closeCursor() as a workaround. But the real question is: Why on earth are buffered queries disabled on your machine? Maybe you're using the old libmysqlclient library instead of mysqlnd. Look for "PDO Driver for MySQL" in phpinfo().

Edited by Jacques1
Link to comment
Share on other sites

 

You're getting an error about an unbuffered query, right?

 

That wasn't the error I think. Give me 10 minutes to fixed my local DNS server, b/s at that moment I don't have an access to mysql :)

 

 

Fatal error: Uncaught exception 'PDOException' with message 'SQLSTATE[28000] [1045] Access denied for user 'lxc'@'lxc.centos.local'

 

 

PS: It's weird, now it's running without any errors. I will try to find tomorrow morning the error from the mysql_error log file and I'll post the result, but there were errors for sure.

Edited by jazzman1
Link to comment
Share on other sites

Yeah, you are right. I found it - "SQLSTATE[HY000]: General error: 2014 Cannot execute queries while other unbuffered queries are active" , then I googled it and found Bill Karwin's solution very useful for me. I think at that time ( more than a mount and something ago ) the server version was 5.1.x, so this testing server version is 5.5.38.  Anyway....sorry, if this is an off topic sor somebody ;)

Link to comment
Share on other sites

Based on Bill Kawin's cite:

 

 

The MySQL client protocol doesn't allow more than one query to be "in progress." That is, you've executed a query and you've fetched some of the results, but not all -- then you try to execute a second query. If the first query still has rows to return, the second query gets an error.

 

Here's my original code tested a mount and half ago. I've got an error without calling PDO::closeCursor().

<?php

ini_set('display_startup_errors',1);

ini_set('display_errors',1);

error_reporting(-1);

$database_options = array(
    PDO::ATTR_EMULATE_PREPARES => false
    , PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
    , PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC
);

$database = new PDO('mysql:host=lxc.centos.local;dbname=test;charset=utf8', 'lxc', 'password', $database_options);

show_prepared_statements();

// *One* prepared statement
$stmt = $database->prepare('SELECT 1 + 1 FROM DUAL');

// ... getting executed 10 times
for($i = 0; $i < 10; $i++){
    
    $stmt->execute();
    
}

$rs1 = $stmt->fetch();

//$stmt->closeCursor();

echo '<br>';

show_prepared_statements();

echo "<br>";

echo '<pre>' . print_r($rs1, true) . '</pre>';

function show_prepared_statements() {

    global $database;

    $statements = $database->query("SHOW SESSION STATUS LIKE 'Com_stmt_%'");
    foreach ($statements as $statement) {
        echo $statement['Variable_name'] . ': ' . $statement['Value'] . '<br>';
    }
}

Result:

 

 

Fatal error: Uncaught exception 'PDOException' with message 'SQLSTATE[HY000]: General error: 2014 Cannot execute queries while other unbuffered queries are active. Consider using PDOStatement::fetchAll(). Alternatively, if your code is only ever going to run against mysql, you may enable query buffering by setting the PDO::MYSQL_ATTR_USE_BUFFERED_QUERY attribute.' in /var/www/html/public_html/pdo/log.php:45 Stack trace: #0 /var/www/html/public_html/pdo/log.php(45): PDO->query('SHOW SESSION ST...') #1 /var/www/html/public_html/pdo/log.php(35): show_prepared_statements() #2 {main} thrown in /var/www/html/public_html/pdo/log.php on line 45

 

 

Mysql server version - 5.5.38

 

Php - 5.4.29

Edited by jazzman1
Link to comment
Share on other sites

This is all I have related to mysql.

[lxc@lxc-centos ~]$ php -i | grep mysql

/etc/php.d/mysql.ini,
/etc/php.d/mysqli.ini,
/etc/php.d/pdo_mysql.ini,
mysql
MYSQL_SOCKET => /var/lib/mysql/mysql.sock
MYSQL_INCLUDE => -I/usr/include/mysql
MYSQL_LIBS => -L/usr/lib64/mysql -lmysqlclient 
mysql.allow_local_infile => On => On
mysql.allow_persistent => On => On
mysql.connect_timeout => 60 => 60
mysql.default_host => no value => no value
mysql.default_password => no value => no value
mysql.default_port => no value => no value
mysql.default_socket => /var/lib/mysql/mysql.sock => /var/lib/mysql/mysql.sock
mysql.default_user => no value => no value
mysql.max_links => Unlimited => Unlimited
mysql.max_persistent => Unlimited => Unlimited
mysql.trace_mode => Off => Off
mysqli
MYSQLI_SOCKET => /var/lib/mysql/mysql.sock
mysqli.allow_local_infile => On => On
mysqli.allow_persistent => On => On
mysqli.default_host => no value => no value
mysqli.default_port => 3306 => 3306
mysqli.default_pw => no value => no value
mysqli.default_socket => no value => no value
mysqli.default_user => no value => no value
mysqli.max_links => Unlimited => Unlimited
mysqli.max_persistent => Unlimited => Unlimited
mysqli.reconnect => Off => Off
PDO drivers => mysql, sqlite
pdo_mysql
pdo_mysql.default_socket => /var/lib/mysql/mysql.sock => /var/lib/mysql/mysql.sock
Link to comment
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.