Mysql_fetch_array just dies silently
-
I was having a problem with php's extract function because it was saying my row from my resultset was not an array, I managed to track my error to here:
$query = 'SELECT * FROM user WHERE username = \'%s\''; //$result = mysql_query($query); echo "<br />"; $result = $databaseRO->query($query, array($_POST['username'])) or die($databaseRO->error()); print($result); $row = $databaseRO->fetchArray($result) or die($databaseRO->error(). "error"); echo "<br /> 2"; extract($row);
It dies and only prints out 'error' the function error() is just a proxy call to mysql_error though the database manager. I can't figure it out, someone care to help me?
-
[edit] <Please ignore this post>. lalala...
-
?
-
my post, not malfist's. answered some stupidtiy there.
-
Most likely the query returns an empty result set. In that case fetchArray() returns null, so you jump to die($db->error()."error"). Since no error occurred in the db, it just dies("error").
BTW, do I smell SQL injection?
-
SQL injection, i guess not, let's assume the database layer correctly escapes the values provided in obj->query.
You should test $row ( check if it is really an array, with ... huu.. is_array() ? )
Or just :
if ( $databaseRo -> fetchArray($result)) {
print_r($row);
} else {
print 'no result dude';
}
-
@MustBeUsersFault said:
SQL injection, i guess not, let's assume the database layer correctly escapes the values provided in obj->query.
Let's not, because PHP is buggy crap and that code has been wrong before.
-
The DatabaseManager class correctly escapes all user input through mysql_real_escape_string(), so sql-injection shouldn't be a problem.
I can't get the $row because mysql_fetch_array() throws an error and dies. I just thought of something. The only reason it would return an empty resultset is if the user wasn't entered into the database, and it doesn't seem to be doing that. Let me look over all my code again.
-
@malfist said:
The DatabaseManager class correctly escapes all user input through mysql_real_escape_string(), so sql-injection shouldn't be a problem.
Until something like CVE-2006-2753 happens (flaw in mysql_real_escape_string).
If you care at all about security, use one of the database libraries that implements prepared queries with parameter marks, not the stupid mysql support that's built-in to php. Or better yet, don't use php.
-
@asuffield said:
@malfist said:
The DatabaseManager class correctly escapes all user input through mysql_real_escape_string(), so sql-injection shouldn't be a problem.
Until something like CVE-2006-2753 happens (flaw in mysql_real_escape_string).
If you care at all about security, use one of the database libraries that implements prepared queries with parameter marks, not the stupid mysql support that's built-in to php. Or better yet, don't use php.
Or better yet, use a real database system instead of MySQL.
-
to be honest, i did make a few projects in php (still do from times to times, but experience gave me a solid framework, in a way).
It's not that buggy, i mean, it's a tool, the final result depends on how you use it.
A clean design, A clean DB design, care about what you have to, never choose the path of easiness, like with every language or tool, i mean, that is our job, isn't it ?
You can build awesome wtfs with ADA, and shiny beautiful pieces of programming masterpieces with php ( i exaggerate a bit, hoping that you see what i mean ).
I assumed everything was correctly escaped because, when i was doing php stuff ( i still maintain a few apps which still work perfectly without any 'security' issue although they are 'internet open' for years now, maybe i'm lucky, maybe i know my stuff, who knows, maybe both, i don't pretend to be a genius, end of the parenthesis ), abstraction layers for databases ( such as PDO ), didn't exist, so we had to build our own ( most of the apps i did had to work whatever the DB system was, Oracle, MSSQL, Mysql, Postgre, pick your own ) and injection stuff were something we had to care about ( who doesn't ).
I'm optimistic, i guess, but the point still is, don't automatically blame the tool when something goes wrong, first, ask yourself ' did i do everything right ?'.
-
@asuffield said:
@malfist said:
The DatabaseManager class correctly escapes all user input through mysql_real_escape_string(), so sql-injection shouldn't be a problem.
Until something like CVE-2006-2753 happens (flaw in mysql_real_escape_string).
If you care at all about security, use one of the database libraries that implements prepared queries with parameter marks, not the stupid mysql support that's built-in to php. Or better yet, don't use php.
CVE-2006-2753 is a MySQL bug, not a PHP bug. C applications using the client library would be affected just the same, and unless your library re-implemented character set-aware escaping (hint: they tend not to), it could happen there as well. Besides, that sort of thing can just as easily happen to any other database out there. For example, PostgreSQL has a very similar flaw described in CVE-2006-2314.
You simply can't guard yourself against bugs not related to your own code, regardless of which programming language and database you use - not unless you re-implement everything on your own, right down to the very OS. It's no different from ANY programming - you have documentation describing the functions available to you, and you rely on those functions working properly. Sure, if you discover something wrong with one particular function, maybe you can re-implement that one, but that's not always an option - for example, if you found a bug in MSSQL's query parser, you'll find it pretty hard to do anything about it, apart from using a different database.
As for the MySQL support in PHP being stupid, most of the mysql-functions in PHP are just simple wrappers around the MySQL client library - including mysql_real_escape_string. They don't actually implement much at all, so yes, I suppose that in that sense, they are indeed stupid. However, once again, PHP isn't really to blame - MySQL gives them an interface, and they use it.
-
@Pidgeot said:
@asuffield said:
@malfist said:
The DatabaseManager class correctly escapes all user input through mysql_real_escape_string(), so sql-injection shouldn't be a problem.
Until something like CVE-2006-2753 happens (flaw in mysql_real_escape_string).
If you care at all about security, use one of the database libraries that implements prepared queries with parameter marks, not the stupid mysql support that's built-in to php. Or better yet, don't use php.
CVE-2006-2753 is a MySQL bug, not a PHP bug. C applications using the client library would be affected just the same, and unless your library re-implemented character set-aware escaping (hint: they tend not to), it could happen there as well. Besides, that sort of thing can just as easily happen to any other database out there. For example, PostgreSQL has a very similar flaw described in CVE-2006-2314.
The whole idea of "escaping" is mostly unnecessary when you use a proper database properly. Bind variables are inherently safe against SQL injection and are better in terms of performance and resource usage, too, on many database systems.
-
@Pidgeot said:
@asuffield said:
@malfist said:
The DatabaseManager class correctly escapes all user input through mysql_real_escape_string(), so sql-injection shouldn't be a problem.
Until something like CVE-2006-2753 happens (flaw in mysql_real_escape_string).
If you care at all about security, use one of the database libraries that implements prepared queries with parameter marks, not the stupid mysql support that's built-in to php. Or better yet, don't use php.
CVE-2006-2753 is a MySQL bug, not a PHP bug. C applications using the client library would be affected just the same, and unless your library re-implemented character set-aware escaping (hint: they tend not to), it could happen there as well. Besides, that sort of thing can just as easily happen to any other database out there. For example, PostgreSQL has a very similar flaw described in CVE-2006-2314.
You simply can't guard yourself against bugs not related to your own code, regardless of which programming language and database you use - not unless you re-implement everything on your own, right down to the very OS. It's no different from ANY programming - you have documentation describing the functions available to you, and you rely on those functions working properly. Sure, if you discover something wrong with one particular function, maybe you can re-implement that one, but that's not always an option - for example, if you found a bug in MSSQL's query parser, you'll find it pretty hard to do anything about it, apart from using a different database.
Preventing SQL injection via escaping is a fundamentally braindamaged approach. You have one piece of code that interprets the query, and another piece of code that predicts how the first one will interpret the query, and modifies the query to stop it from doing that. Unless you can manage to keep the two perfectly in sync, you have security holes. The chances of getting this right are very slim. Historically they have done a very bad job of getting it right, and there's no particular reason to think that it's any better now.
The correct method is to stop passing the query as a flat string, and segregate the tainted data from the executable code properly. All the important databases (and mysql) implement this using parameter marks in prepared queries. All the major languages except PHP support (and very strongly recommend that you use) this. Yes, other C applications that were as braindamaged as PHP would also suffer from this flaw - but most major ones are not this poorly designed.
As for the MySQL support in PHP being stupid, most of the mysql-functions in PHP are just simple wrappers around the MySQL client library - including mysql_real_escape_string. They don't actually implement much at all, so yes, I suppose that in that sense, they are indeed stupid. However, once again, PHP isn't really to blame - MySQL gives them an interface, and they use it.
They implement only the stupid subset of the mysql library, which operates on raw strings. Most notably, PHP's builtin mysql functions completely lack prepared query support. PHP is entirely to blame for this.
-
They implement only the stupid subset of the mysql library, which operates on raw strings. Most notably, PHP's builtin mysql functions completely lack prepared query support. PHP is entirely to blame for this.
The mysqli extension, designed for mysql >= 4.1.3 supports all that and some more. I don't think mysql itself supported those functions before that.
-
@Sunstorm said:
They implement only the stupid subset of the mysql library, which
operates on raw strings. Most notably, PHP's builtin mysql functions
completely lack prepared query support. PHP is entirely to blame for
this.The mysqli extension, designed for mysql >= 4.1.3 supports all that and some more.
And there's no excuse for not using it (or one of the other two or three that do the same). Just don't use the damned builtins (or preferably, use anything other than PHP).
-
Okay, this is the query function from my DatabaseManager object. Maybe you all can stop bickering over it. I'm not to worried about sql-injection. The user that accesses the DB doesn't have all privileges, and plus, it's just a message board for a local club from college.
For those that hate PHP, have you ever really used it? Properly? It's a very nice language when used correctly, it does have it's flaws, as do all other languages. PHP can do a lot more than Ruby on Rails and several other web development languages.
The number one reason languages are no good, or 'bad' are their users, not it, usually.
-
I'd prefer any other language that didn't implicitly perform a substitution operation on the program text... necessitating a full parse and additional series of in-memory operations per execution.
Wake me up when the PHP team invents a language specification that is Java to JSP. Which is totally backwards, but I digress.
Yes, I know about Zend. No, that shouldn't be necessary.
-
Just realized, I didn't post the code:
/** * The query to use. * @param $query holds the query with %s as the values * @param $params is an array that holds the values that replace the %s's */ function query($query,$params=array()) { for ($i=0;$i<count($params);$i++) { $params[$i] = mysql_real_escape_string($params[$i]); } $prep_query = vsprintf($query,$params); //echo $prep_query, "<br />"; //for debuging return mysql_query($prep_query); }
-
Well, first off I'd try getting out a copy of the query string after it's been built in there, and manually running it in the mysql client and see if that even works. Could have a syntax error or something.
And not knowing how this 'DatabaseManager' works, are you sure that $databaseRO->error() actually returns the error string? It might just return a boolean, and the actual error message is retrieved using another function.
Oh, and a minor optimization: PHP will not cache the count($params) value in the loop, it'll be recalculated each time (since it's possible the array might be changed within the loop). No biggie if you're passing in a just a few parameters, but eventually something like that'll melt down your CPU with wasted cycles.
-
@asuffield said:
Preventing SQL injection via escaping is a fundamentally braindamaged approach. You have one piece of code that interprets the query, and another piece of code that predicts how the first one will interpret the query, and modifies the query to stop it from doing that. Unless you can manage to keep the two perfectly in sync, you have security holes. The chances of getting this right are very slim. Historically they have done a very bad job of getting it right, and there's no particular reason to think that it's any better now.
The correct method is to stop passing the query as a flat string, and segregate the tainted data from the executable code properly. All the important databases (and mysql) implement this using parameter marks in prepared queries. All the major languages except PHP support (and very strongly recommend that you use) this. Yes, other C applications that were as braindamaged as PHP would also suffer from this flaw - but most major ones are not this poorly designed.
The last time I looked at the C-API (admittedly years ago) there was no support for parameterized queries. You had to use the escape functions. Everything was returned as a string, so if you selected a number, you had to convert it from a string to a number yourself after you got it back from the database. Unless I just completely misread the API documentation.
Dave
