Doc #60398 [Com]: mysql_real_escape_string description is wrong and decieving
| From: | col dot shrapnel at gmail dot com | Date: | Mon, 05 Dec 2011 15:07:28 +0000 |
| Subject: | Doc #60398 [Com]: mysql_real_escape_string description is wrong and decieving | ||
| References: | 1 | Groups: | php.doc.bugs |
| Request: | Send a blank email to doc-bugs+get-7581@lists.php.net to get a copy of this message | ||
Edit report at https://bugs.php.net/bug.php?id=60398&edit=1
ID: 60398
Comment by: col dot shrapnel at gmail dot com
Reported by: col dot shrapnel at gmail dot com
Summary: mysql_real_escape_string description is wrong and
decieving
Status: Wont fix
Type: Documentation Problem
Package: Documentation problem
Operating System: irrelevant
PHP Version: Irrelevant
Block user comment: N
Private report: N
New Comment:
Errr... I beg my pardon, but are you sure you commented the right bug?
First, I didn't say that the whole extension needs to be deprecated and removed. I never
mentioned the extension at all, but one function only.
Next, I see no logic in your statement. If it was already deprecated - yes, there would be no point
in improving it, I admit that. But as long as it's still active - why not to improve the
documentation a bit? At least for ones who have to run a legacy code?
I'd even dare to say that there is nothing wrong neither with mysql extension nor escaping. It
has it's bad reputation mainly because of the improper use, partially inspired by the
documentation page I am referring to.
Previous Comments:
------------------------------------------------------------------------
[2011-12-03 04:36:06] frozenfire@php.net
Hi. Let me first say that I agree with you wholeheartedly. The mysql extension
needs to be deprecated and removed as soon as possible, because it encourages
new programmers to make bad design decisions in favour of quick, sloppy code.
However, this is not a documentation problem. So long as the project retains the
mysql extension as an active extension, and does not provide any notice-type
errors warning against its use, there is nothing to be done from the
documentation perspective.
In short, yes you're right, but no we can't do anything about it.
------------------------------------------------------------------------
[2011-11-27 10:26:38] col dot shrapnel at gmail dot com
Description:
------------
---
From manual page: http://www.php.net/function.mysql-real-escape-string#refsect1-
function.mysql-real-escape-string-description
---
I am writing in account of the mysql_real_escape_string() description, which
current phrasing is erroneous and decieving, leading thousands of PHP
programmers to confusion and make them writing the code that actually _allows_
injection.
It says at the moment
---
This function must always (with few exceptions) be used to make data safe before
sending a query to MySQL.
---
Which is obviously wrong, as the function doesn't make data whatever "safe".
And "few exceptions" statement is not an excuse as it explains nothing.
Based on this very description, many people having an idea of injection
protection limited to just "escape all your data" and actually allow an
injection as a result.
I insists on the different phrasing, says (with obvious grammar or styling
check):
---
This function must always be used to process every string (i.e. piece of data
enclosed in the single quotes) added to the query.
Note that this function doesn't make any data "safe" as it's just escaping
special characters in the strings only and thus it is useless to protect other
data types, such as numbers or identifiers.
---
Same goes for the note, saying
---
If this function is not used to escape data, the query is vulnerable to SQL
Injection Attacks.
---
"data" again!
So, it is just false statement, as even if the function were used, there are
circumstances under which your query remains vulnerable.
Also, in account of the only purpose of this function, in should be explicitly
noted that only prior call to mysql_set_charset() will make the
mysql_real_escape_string different from mysql_escape_string() - i.e. make it "
taking into account the current character set of the connection".
Test script:
---------------
$data = "1 union select password from users"
$data = mysql_real_escape_string($data);
$sql = "SELECT title FROM news WHERE id=$data";
Expected result:
----------------
The $data become "safe" and stopped injection.
Actual result:
--------------
The code above didn't make the data "safe" and didn't stop the injection.
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=60398&edit=1