0

I have a strange error: I have that simple code:

$id = strip_tags($_SESSION["infos_profile_id"]);
$id_friend = strip_tags($_POST["update_user_chat_every_5_second"]);
$q = $bdd->query('SELECT * FROM message WHERE id_sender = '.$id_friend.' AND id_send_to = '.$id.' AND message_read = "0"');

It work fine on mysql.

But after hosting my website on mariadb server, It see that error.

PHP Fatal error: Uncaught exception 'PDOException' with message 'SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'AND message_read = "0"' at line 1' in

I have done everything to solve but I can't find where is really the error from.

Any help to solve that error ?

Thanks.

ankalagba
  • 94
  • 1
  • 8
  • You cannot intermix different mysql apis. – Funk Forty Niner Feb 29 '20 at 16:53
  • I don't think that's the issue. MariaDB and MySQL aren't different APIs nor does the OPs example code suggest he's intermixing APIs. And the error isn't related to the one on the linked question (which is trying to use two different php APIs, mysql and mysqli) – Anthony Feb 29 '20 at 17:06
  • 2
    **WARNING**: Whenever possible use **prepared statements with placeholder values** to avoid injecting arbitrary data in your queries and creating [SQL injection bugs](http://bobby-tables.com/). These are quite straightforward to do in [`mysqli`](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) and [PDO](http://php.net/manual/en/pdo.prepared-statements.php) where any user-supplied data is specified with a `?` or `:name` indicator that’s later populated using `bind_param` or `execute` depending on which one you’re using. – tadman Feb 29 '20 at 17:07
  • I would try not wrapping the `0` in quotes. Your MariaDB is probably set to some mode where it's treating number fields more strictly. – Anthony Feb 29 '20 at 17:08
  • `strip_tags` does *absolutely nothing* to prevent SQL injection. It is completely useless in this context. – tadman Feb 29 '20 at 17:08
  • Calendar reads 2020 and developers *still* do not [parameterize](https://stackoverflow.com/q/60174/1422451) queries especially on user input of `$_POST` variables. – Parfait Feb 29 '20 at 17:10
  • 1
    @Parfait The PHP world is filled with abject poverty and misery, which is astounding considering how many great tools there are that are free and easily downloaded, just there for the taking. It's probably because these great tools are buried under heaps of ignorant YouTube "tutorials" and landfills like w3schools. – tadman Feb 29 '20 at 17:13
  • @Anthony Why did you reopen this? – Funk Forty Niner Feb 29 '20 at 17:17
  • Why don't you just post the output of echo "'SELECT * FROM message WHERE id_sender = '.$id_friend.' AND id_send_to = '.$id.' AND message_read = "0"" ? Nobody knows the content of your SESSION and POST variables. – Georg Richter Feb 29 '20 at 17:29
  • @Georg Richter they are Integer of course. – ankalagba Feb 29 '20 at 17:43
  • @tadman Nobody birth professionnel programmer. – ankalagba Feb 29 '20 at 17:47
  • It seems to be working if I use prepared query. I have 50 query like that, that mean I had to make prepared query all these 50 query ? what is a worst solution. It's why I asked that issue to know how to solve with non prepared query. I'm Always looking for solution. Sorry for my bad english. – ankalagba Feb 29 '20 at 18:08
  • I'm not criticizing you, I'm just disappointed that the PHP community doesn't step up and solve this problem so people like you that are just trying to learn aren't given terrible advice out of the gate. Other languages, like Python, Ruby, and even Java, all have solutions to this problem that people find first, and as a community work to stomp out dangerous practices at the earliest opportunity. – tadman Feb 29 '20 at 18:27
  • 1
    @tadman I had badly understood your english. So thanks guys for your comments. – ankalagba Feb 29 '20 at 18:58
  • 1
    Whenever you have an error like this it is usefull to log the the actually generated SQL to see what you are sending to the database and then examine it as such. Other than that as others you cannot simply create SQL by using variabjes posted from forms without making sure they are not secure, by either using parameters and bind values with provided functions/methods or make sure to parse the input values using provided mysqli_real_escape_string function or similar one to prevent SQL injection. – MaxT Feb 29 '20 at 20:53
  • @funkyfortyniner - stated in earlier comment. I don't believe it is related to that question it was closed against. – Anthony Mar 02 '20 at 22:03

1 Answers1

0

I agree with the comment from @MaxT above -- you should echo your SQL query after interpolating all variables into it. It's too difficult to debug when you're looking at code that formats an SQL query, instead of the query itself.

Comments are also correct that strip_tags() is not useful for SQL injection protection.

Query parameters are the best protection against SQL injection, and they help you avoid syntax errors too.

Here's what it would look like for your code:

$id = $_SESSION["infos_profile_id"];
$id_friend = $_POST["update_user_chat_every_5_second"];
$sql = 'SELECT * FROM message WHERE id_sender = :id_friend AND id_send_to = :id AND message_read = 0';
$q = $bdd->prepare($sql);
$q->execute( ['id'=>$id, 'id_friend'=>$id_friend] );

It's really very easy!

Bill Karwin
  • 538,548
  • 86
  • 673
  • 828
  • In all my project, I have 604 query which are non prepared. So do I have to change all these 604 query ? Oooh my head, please tell me I don't have to. – ankalagba Mar 01 '20 at 09:01
  • You don't have to change a query unless it interpolates PHP variables. If the query is just a plain string with no variables, there's no need to change it. You should review every query to make sure it's safe, but you might only need to change a subset of them. – Bill Karwin Mar 01 '20 at 17:50
  • Think of it this way: Suppose you were an electrician, and your supervisor found that you had installed 604 junction boxes in a way that risked causing a house fire. If you whine, "do I have to fix all 604 of them? please tell me I don't have to," then you are not taking responsibility for safety like a professional person should. I would not be surprised if your supervisor sacked you for that comment. – Bill Karwin Mar 01 '20 at 17:55
  • SQL injection is a serious flaw, and it causes data breaches regularly. Read https://codecurmudgeon.com/wp/sql-injection-hall-of-shame/ where a blogger has collected news reports of SQL injection cases. – Bill Karwin Mar 01 '20 at 18:01