[RESOLVED] Secure Query

[TheCase]

Hi,

Whats more secure or is there no difference?

$userteamcheck = "SELECT * FROM smf_members WHERE memberName='$username'" or trigger_error("Query: $userteamcheck\n<br />MySQL Error: ". mysql_error());
$userteamcheck2 = mysql_query ($userteamcheck);
while ($userteamcheck3 = mysql_fetch_assoc($userteamcheck2)) {
$teamnamme = $userteamcheck3['team']; 
$id= $userteamcheck3[id]; 

$userteamcheck = "SELECT team, id FROM smf_members WHERE memberName='$username'" or trigger_error("Query: $userteamcheck\n<br />MySQL Error: ". mysql_error());
$userteamcheck2 = mysql_query ($userteamcheck);
while ($userteamcheck3 = mysql_fetch_assoc($userteamcheck2)) {
$teamnamme = $userteamcheck3['team']; 
$id= $userteamcheck3[id]; 

I guess the second one is more secure as there is no willcard, so does this mean no one can run an sql injection, I also guess its slower though

Thanks

[laserlight]

Whats more secure or is there no difference?

They are both as secure, or as insecure, as the case may be.

I guess the second one is more secure as there is no willcard, so does this mean no one can run an sql injection, I also guess its slower though

On the contrary, the second version is more efficient since it selects only those columns that are needed.

However, the question of SQL injection is not concerned with what columns are selected, but what values are used with the query. In this case, the value used with the query comes from $username. If you have not correctly escaped $username with say, [man]mysql_real_escape_string/man before using it to construct the query, then you are vulnerable to SQL injection.

By the way, you are using trigger_error() in the wrong place. It should probably be:

$userteamcheck2 = mysql_query($userteamcheck)
    or trigger_error("Query: $userteamcheck\n<br />MySQL Error: ". mysql_error());

It does not make sense to trigger an error when building the SQL statement, because if there is an error at that point, it is entirely your fault as the developer of the script.

[TheCase]

Thanks alot. The $username is coming from this

$username = $context['user']['name'];

The $context is defined from another file wich is added by require_once(). The query now stands like this

$userteamcheck = "SELECT team FROM smf_members WHERE memberName='$username'";
$userteamcheck2 = mysql_query ($userteamcheck) or trigger_error("Query: $userteamcheck\n<br />MySQL Error: ". mysql_error()); 
while ($userteamcheck3 = mysql_fetch_assoc($userteamcheck2)) {
$teamnamme = $userteamcheck3['team']; 

[laserlight]

In that case, to avoid SQL injection, you should change

$userteamcheck = "SELECT team FROM smf_members WHERE memberName='$username'";

to:

$userteamcheck = sprintf("SELECT team FROM smf_members WHERE memberName='%s'",
    mysql_real_escape_string($username));

Incidentally, instead of using $userteamcheck, $userteamcheck2 and $userteamcheck3, it may be better to use $query (or $sql), $result, and $row, respectively.

[TheCase]

Thanks