[CTphpnwb]

Then use mysqli, but be sure to use prepared statements. With both PDO and mysqli it's possible to do something like:

$query = "SELECT * FROM table WHERE id='$id'";

which will negate the advantage of prepared statements by making data (probably user-supplied) part of the query!

[adn258]

CTphpnwb, on 04 October 2012 - 07:32 PM, said:

Then use mysqli, but be sure to use prepared statements. With both PDO and mysqli it's possible to do something like:

$query = "SELECT * FROM table WHERE id='$id'";

which will negate the advantage of prepared statements by making data (probably user-supplied) part of the query!

Well ok sorry I might want to start some PDO stuff what's the deal though with closing connections and freeing objects in PDO or do you even have to worry about that? Some people have said PDO should be set to null when done? What about objects

That said I'm using fetch_assoc here in this PDO prepared statement WOULD THIS BE SAFE FROM SQL INJECTION? I'm still having issues getting that I thought FETCH_ASSOC was dangerous even with PDO?

$query = "SELECT * FROM people WHERE id= ?";
		$sth = $dbh->prepare($query);
		$sth->bindParam(1, $this->id,PDO::PARAM_INT);
		$sth->execute();
		$result = $sth->fetch(PDO::FETCH_ASSOC);
		$dbh = NULL;
		return $result;

This post has been edited by adn258: 04 October 2012 - 08:18 PM

[CTphpnwb]

Why would fetching an associative array from the database be dangerous? The query is where the vulnerability lies in SQL, and as long as the query contains no user supplied information it cannot be compromised.

The only issue I see — and it's not a security issue — is using SELECT * instead of field names. It requires more work of the database to return every field, so you should only request the fields you need. Of course, if you need them all, it's not an issue.

Oh, and opening closing connections is similar to old fashioned MySQL. Generally, you should open one, do your business, and then let it expire with the session.

This post has been edited by CTphpnwb: 04 October 2012 - 08:30 PM

[adn258]

CTphpnwb, on 04 October 2012 - 08:26 PM, said:

Why would fetching an associative array from the database be dangerous? The query is where the vulnerability lies in SQL, and as long as the query contains no user supplied information it cannot be compromised.

The only issue I see — and it's not a security issue — is using SELECT * instead of field names. It requires more work of the database to return every field, so you should only request the fields you need. Of course, if you need them all, it's not an issue.

Oh, and opening closing connections is similar to old fashioned MySQL. Generally, you should open one, do your business, and then let it expire with the session.

Thanks CTphpnwb you've been amazingly helpful too me and kind...I really appreciate it +++1. So as long as the parameters are binded and user values not directly supplied into the statement you're safe? Okay I'm a little confused on the let it expire idea? I many times a use a lot of functions that return something or do something. Before returning from the function or if it's a void function before the last brace shouldn't you null or close the connection? That's what I've always done but maybe that's excessive?

Also last but not least what about pdo objects? Are objects automatically freed?

This post has been edited by adn258: 04 October 2012 - 08:38 PM

[CTphpnwb]

Unless you're using persistent connections, and if you were you would know it, the connection closes when the script ends. As for objects, they and everything else are freed when the script ends.

Think of PHP as batch processing. You run a script and when it's done, it's done. That's why you need session variables if you want information to be retained without resubmitting it from the browser over and over again.

[adn258]

CTphpnwb, on 04 October 2012 - 08:48 PM, said:

Unless you're using persistent connections, and if you were you would know it, the connection closes when the script ends. As for objects, they and everything else are freed when the script ends.

Think of PHP as batch processing. You run a script and when it's done, it's done. That's why you need session variables if you want information to be retained without resubmitting it from the browser over and over again.

Thanks CT so you're saying even with mysqli using free is almost a waste of time? I get it....okay one final question because I think I'm getting this down. In one part of my script I use different bind_params over and over again in a foreach to remove an array of comment ID's and this is checked by the session user to make sure the user can ONLY delete their own comments (this was my security mistake last time) it was stupid but basically I set it up so users can check mark their comments and hit delete but since it wasn't checked the logged in user someone could $_POST any id they wanted. Okay anyway is there anything wrong with using a foreach like below with bindParams and Execute() ?

$query = "DELETE FROM comments WHERE ID=? AND poster=?;";
$stmt = $dbh->prepare($query);
$stmt->bindParam(2,$logged_user,PDO::PARAM_STR);
$i = 0;
foreach ($comments as $num)
{
   $stmt->bindParam(1,$num,PDO::PARAM_INT);
   if ($stmt->execute())
   {
	 $i++;   
   }
}
$dbh = NULL;
echo " Comment(s) Removed " . $i);
exit();

[Dormilich]

adn258, on 05 October 2012 - 04:16 AM, said:

But I read somewhere that you can't use the functions I was talking about in mysqli without some like mysdli driver in the server or something?

without a proper database driver (e.g. mysqlnd) PHP cannot work with any database.

adn258, on 05 October 2012 - 04:16 AM, said:

Seriously I tried to use get_result it doesn't work function is undefined.

maybe there’s something wrong with your code. maybe your host doesn’t support MySQLi. but we can’t know. you could post your code, you could check availability in phpinfo(), we can’t do that for you.

[Dormilich]

adn258, on 05 October 2012 - 07:28 AM, said:

In one part of my script I use different bind_params over and over again in a foreach to remove an array of comment ID's and this is checked by the session user to make sure the user can ONLY delete their own comments (this was my security mistake last time) [...] Okay anyway is there anything wrong with using a foreach like below with bindParams and Execute() ?

$query = "DELETE FROM comments WHERE ID=? AND poster=?;";
$stmt = $dbh->prepare($query);
$stmt->bindParam(2,$logged_user,PDO::PARAM_STR);
$i = 0;
foreach ($comments as $num)
{
   $stmt->bindParam(1,$num,PDO::PARAM_INT);
   if ($stmt->execute())
   {
	 $i++;   
   }
}
$dbh = NULL;
echo " Comment(s) Removed " . $i);
exit();

the problem here is that the sequence is wrong.
1) you prepare the statement
2) you bind the values/parameters (exactly as many as you have)
3) you execute the statement (as often as desired)

what you want to do (change $num accordingly) you already do without realising. bindParam() binds a variable (the variable itself, not its value, kind of like a pointer), hence you would only need to iterate over $num. (ex.)

$query = "DELETE FROM comments WHERE ID=? AND poster=?;";
$stmt = $dbh->prepare($query);
$stmt->bindParam(1, $num, PDO::PARAM_INT);
$stmt->bindValue(2, $logged_user, PDO::PARAM_STR);

foreach ($comments as $num)
{
   if ($stmt->execute())
   {
	 $i++;   
   }
}

echo " Comment(s) Removed " . $i);

This post has been edited by Dormilich: 04 October 2012 - 11:21 PM

[adn258]

I just didn't think of doing it that way...experience experience experience I take it? Thanks for your support the way I had it worked but I'm assuming it's way less efficient?

This post has been edited by Dormilich: 05 October 2012 - 01:15 AM
Reason for edit:: removed unnecessary quote

[Dormilich]

definitely. if you have a loop construct, always try to execute as less functions as possible within.

[cilaes]

Your original problem didn't have to do with not using prepared statements, though. Just bad model design.

DELETE FROM `table` WHERE `ID`='$id' && `user_ID`='$uid'