Debugging help needed

[the182guy]

I think I understand what you are saying, but doesn't the previous code sanitize $userid?

    $userid=sanitizeFromPost('userid');
    if (strlen($userid)<5) {
        $msg .= 'User ID should be 5 or more than 5 char length.<br>';
    }
    if (mysql_num_rows(mysql_query("SELECT user_id FROM member_tbl WHERE user_id = '$userid'"))) {
        $msg .= 'Userid already assigned. Please select another userid.<br>';
    } 

I will make modifications later this evening and post the new code.

mysql_real_escape_string() should not be used to sanitize numerical values, it will do nothing to prevent SQL injection when special hex values are used.

Numerical data should be casted as the expected type, so for example a user id:

$userId = (int)$_POST['userid'];

// $userId is now an integer and safe to put in SQL.

[CSU-Bill]

I may be missing something, but I don't think I want to cast this as int.

Maybe I should change and use something other than userid. This is, in this case, really a user name. In this case, casting your user id of the128guy would not let me store the entire ID.

[CSU-Bill]

StarLion,

The first line of code in your example reads:
if(!$result = mysql_query($sql)) {

Should I replace the ($sql) with something like:
("SELECT user_id FROM member_tbl WHERE user_id = '$userid'")

[StarLion]

Personally i keep my query string seperate as a variable. It makes it easier to check it (echo $sql), and it's sort of a half-step towards prepared statements.

as the182guy said, sanitize numerics by casting them, but your query structure indicates that you arnt using a numeric value, you're using a string. (Numerical values dont get quoted in SQL.), which is why i suggested escape_string. And no, it's not a solves-all, but it's at least sanitization against the most basic attacks.

[CSU-Bill]

This is the code I currently have and it give me an error:

    $userid=sanitizeFromPost('userid');
    if (strlen($userid)<5) {
        $msg .= 'User ID should be 5 or more than 5 char length.<br>';
    }
    if (mysql_num_rows(mysql_query("SELECT user_id FROM member_tbl WHERE user_id = '$userid'"))) {
        $msg .= 'Userid already assigned. Please select another userid.<br>';
    }                    
    if (!$result = mysql_query($sql)) {
      //There was an error in our query and it returned FALSE.
          echo mysql_error();
    } elseif(mysql_num_rows($result) == 0) {
      //The query executed successfully (it returned a Result Resource), but had no results to return.
    } else {
      //The query both executed successfully, and returned at least 1 row of data.
} 

The error is:
Notice: Undefined variable: sql inmember_info_ck.php on line 55

Line 55 is: if (!$result = mysql_query($sql)) {

So do I need to put in a line that reads:
$sql = (mysql_num_rows(mysql_query("SELECT user_id FROM member_tbl WHERE user_id = '$userid'")

And then change my query to be:
if ($sql) {

[StarLion]

$sql = "SELECT user_id FROM member_tbl WHERE user_id = '$userid'";

[CSU-Bill]

StarLion,

Maybe I am starting to understand. Here is what I now have:

    $sql = "SELECT user_id FROM member_tbl WHERE user_id = '$userid'";
    if (!$result = mysql_query($sql)) {
      //There was an error in our query and it returned FALSE.
          echo mysql_error();
    } elseif(mysql_num_rows($result) == 0) {
      //The query executed successfully (it returned a Result Resource), but had no results to return.
    } else { $msg .= 'Userid already assigned. Please select another userid.<br>';
      //The query both executed successfully, and returned at least 1 row of data.
} 

Do I have this close to being correct?

[StarLion]

Yes; though if you're not going to do anything in the num_rows = 0 case, change if around so the elseif reads not-equal 0, and put your message in there, and then leave off the else clause.

[CSU-Bill]

Thanks. I think I get it now and will test it when I get back home in about 8 hours.