mysql_num_rows() expects parameter 1 to be resource, boolean given AFTER checking success

Question

I am having the issue 'mysql_num_rows() expects parameter 1 to be resource, boolean given'

I realise this means that the query has failed. However, I am first doing a check to see if the query was successful and within that if, i am getting this error. I am not sure why I would get into the if in the first place if the query failed?? Also i have tried echoing out the query and pasting into phpmyadmin and it runs successfully..

Code below:

$checkloginEmp = "CALL login('".$Email."','".$Password."', '".$NotificationID."', '".$Token."', @numRowsParam, @userIDParam);";
                //echo $checkloginEmp;

                if ($check = mysql_query($checkloginEmp)){
                    //echo $check;

                    if(mysql_num_rows($check) == 1) {
                       //retrieve info. issue points to here
                       $status= 'OK';
                       $message= null;
                    }
                    else{
                       $status= 'Not OK';
                       $message= "Invalid email/pass combination";
                    }
                 }

shouldn't be happening. if the query failed, then `$check` becomes false and the overall `if()` false as well. in php, the result of an assignment is the value being assigned... — Marc B, May 27 '15 at 15:06
[Your script is at risk for SQL Injection.](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) Please, [stop using `mysql_*` functions](http://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php). They are no longer maintained and are [officially deprecated](https://wiki.php.net/rfc/mysql_deprecation). Learn about [prepared statements](http://en.wikipedia.org/wiki/Prepared_statement) instead, and consider using PDO, [it's not as hard as you think](http://jayblanchard.net/demystifying_php_pdo.html). — Jay Blanchard, May 27 '15 at 15:07
check mysql_error()? false means failure, so an error should be set somewhere... — Marc B, May 27 '15 at 15:08
@MarcB I have mysql_error() in the else for the first if, but since i am getting into the first if, I don't know where I am supposed to check mysql_error()? — user2363025, May 27 '15 at 15:10
@Jay Blanhard it is at risk even though I'm using stored procedures with invoker rules? — user2363025, May 27 '15 at 15:10
`if ($check = ...) { die(mysql_error()); }`, basically. check it inside the if(). I doubt it'll help, since $check should be ok at that point, otherwise the true clause of the if() would never have triggered. — Marc B, May 27 '15 at 15:11
you are using `mysql_` to connect with, correct? and not `mysqli_` or PDO. I had to ask. — Funk Forty Niner, May 27 '15 at 15:11
Yes - totally at risk although any attempt would be likely to fail because the `CALL` would return errors. — Jay Blanchard, May 27 '15 at 15:12
then there's no error at that point. start moving it around and see if/where the error occurs... — Marc B, May 27 '15 at 15:13
You shouldn't be checking `mysql_error` at all. Anywhere. Instead, you **should be** using either **`mysqli`** or **`PDO`**. Why are you spending time developing an application using a deprecated interface, when you could be spending that time using a supported interface? It boggles the mind. And why are you writing an application that appears to be so blatantly **vulnerable** to **SQL Injection**? You **should be** using a **prepared statement** with **bind placeholders**. — spencer7593, May 27 '15 at 15:14
@MarcB no error inside the first if and the error inside the second if is the one described in the question. There is no code in between.. — user2363025, May 27 '15 at 15:16
Then there's no way it can go from non-false inside the `if()` to being false when you call num_rows... — Marc B, May 27 '15 at 15:17
@MarcB that's what's happening though, there is no extra code in between that i am leaving out.. — user2363025, May 27 '15 at 15:19
I guess start rejiggering the code. `$check = mysql(); if ($check) { ...} ` and so on. break it down into individual smaller operations, are put a ton of debugging output before/after each one. — Marc B, May 27 '15 at 15:20
plus http://php.net/manual/en/function.error-reporting.php and check your procedure. Plus, [another comment of mine...](http://stackoverflow.com/questions/30486308/mysql-num-rows-expects-parameter-1-to-be-resource-boolean-given-after-checkin#comment49051359_30486308) that's not been answered, but is probably irrelevant; *is it?* — Funk Forty Niner, May 27 '15 at 15:22
@spencer7593 I thought it was ok to bind parameters to a stored procedure — user2363025, May 27 '15 at 15:22
@Fred-ii- sorry I attempted to reply but stackoverflow was limiting my comments, i had to wait. Yes I am using mysql_ to connect — user2363025, May 27 '15 at 15:24
ah, no worries; thanks. Not sure if this will even help, but try adding error reporting to the top of your file(s) right after your opening PHP tag for example ` — Funk Forty Niner, May 27 '15 at 15:27
@spencer7593 binding parameters to a stored procedure is not safe? Can you provide an example of how an attack would succeed? — user2363025, May 27 '15 at 15:40
@Fred-ii- it only fails when I provide an invalid pass/email combo. It succeeds when I give correct credentials. Any idea why? — user2363025, May 27 '15 at 15:48
sorry, no idea. However, you could try a conditional `isset()` or `!empty()` for the variables. — Funk Forty Niner, May 27 '15 at 15:49

spencer7593 · Accepted Answer · 2015-05-27T16:14:51.260

1

The code in the question appears to be attempting to return a resultset from a stored procedure.

(If that procedure isn't returning a resultset, then there's no reason we'd be using mysql_num_rows.)

In order to return a resultset from a procedure, the CLIENT_MULTI_STATEMENTS option has to be enabled through the client_flags parameter on the call to mysql_connect. The option is enabled by a flag value of 65536.

Without that option, it's not possible to return a resulset from a stored procedure.

WARNING:

Opening a connection that enables multiple statements throws wide open the doors for a whole host of NASTINESS for applications that are vulnerable to SQL Injection.

NOTES:

Is it necessary to return a resultset from the stored procedure? Do you need multiple rows returned, or could you return scalar values using OUT procedure parameters?

As I noted in my previous comment, this code is using a deprecated interface. New development should use either mysqli or PDO.

And incorporating potentially unsafe values in SQL text makes the application vulnerable to SQL Injection.

EDIT

Q: @spencer7593 binding parameters to a stored procedure is not safe? Can you provide an example of how an attack would succeed?

A: That's a different question.

The example code does is not "binding parameters". It's incorporating values of PHP variables into the text of a SQL statement. (What we don't see in the code example is if those values have been properly escaped. And without seeing that, I'm going assume that they haven't been, both because that's the most common pattern that we see; and because it's the safer assumption. We assume that the values are unsafe, until it's demonstrated that they aren't.

For examples of attack vectors and exploits of SQL Injection, I recommend the OWASP project. https://www.owasp.org/index.php/SQL_Injection.

If you enable the option that allows multiple statements to run (which you need, if you are going to return a resultset from a stored procedure, here's an example of a value we might attempt to get passed in for $Token

fee', @fi, @fo); DROP TABLE users; --

Little Bobby Tables Exploits of a Mom XKCD https://xkcd.com/327/

edited May 27 '15 at 16:14

answered May 27 '15 at 16:06

spencer7593

103,596
14
107
133

first off, thank you very much for your detailed response! from now on, I will definitely use mysqli. At the minute I am stuck with mysql .I created my stored procedures with phpmyadmin 'routines', so there are parameters slotted into the stored procedure. The only bit which I left out in the original question was that i do use mysql_real_escape_string around any posted variables. I tried the example you provider of the injection attack and it did not work. Does this mean I am safe? – user2363025 May 27 '15 at 18:35
Using `mysql_real_escape_string` mostly prevents the easiest vulnerability to exploit: including a quote character in a value. (I say mostly prevents, because there's still a possibility for a quote character to get snuck past if there's a characterset translation happening. That is, there may be some extended characters that "pass" the escape string check, but later get mapped to a quote when a characterset translation occurs. Using `mysql_real_escape_string` is imperative. Doing that does close up one gaping wide attack vector. But it doesn't close every avenue. – spencer7593 May 27 '15 at 18:51
Again, I recommend the OWASP project as a starting point. [https://www.owasp.org/index.php/SQL_Injection_Prevention_Cheat_Sheet](https://www.owasp.org/index.php/SQL_Injection_Prevention_Cheat_Sheet) This isn't exhaustive, but it does cover the minimum *every* developer should know. And should give you some understanding as to why you should use **prepared statements** with **bind placeholders**. – spencer7593 May 27 '15 at 18:54
If your stored procedure isn't returning a resultset (that is, it's not running a bare `SELECT`), then you wouldn't want to use `mysql_num_rows`, since that function only makes sense with a resultset object. – spencer7593 May 27 '15 at 18:56
The stored procedure does return a result set i.e. all the profile info of the user logging in. Which it does if I give correct credentials but if I give a wrong password, i'm getting the error described above – user2363025 May 27 '15 at 19:10
Are prepared statements possible with mySQL? I can't find any examples. They all use mySQLi – user2363025 May 27 '15 at 19:38
No. The deprecated `mysql_` interface does *not* support prepared statements. The replacement interface `mysqli_` *does* support prepared statements, while paying homage to the mysql interface, with functions that are very similar. Note that the `mysqli` interface is specific to MySQL, while `PDO` supports a whole host of other databases in addition to MySQL. – spencer7593 May 27 '15 at 20:36
Off I go to change 30 webservices ;) thanks! – user2363025 May 27 '15 at 21:31
@user2363025: ... or just do what most developers do, ignore the issues, pretend you don't know what "deprecated" means, and pretend that SQL Injection is something you deal with "later". Once you have all the code developed, test and implemented using a deprecated interface and with gaping wide SQL Injections. Just figure it'll be easier to break bad habits and the work will be less painful later, than it is now. (I think converting existing code to mysqli is "easier" than converting to PDO. But if we're starting from scratch, PDO is a better choice in a lot of cases. – spencer7593 May 27 '15 at 22:56

mysql_num_rows() expects parameter 1 to be resource, boolean given AFTER checking success

1 Answers1