Do I need to escape the return of mysqli_error()?

[Mig21]

Hi

 

I find it very strange that I can't find an answer to this question.

 

In my code I have queries where I use mysqli_real_escape_string() to make sure there is no SQL injection happening.

 

But then I just call mysqli_error() and dump its output into standard out, without escaping it using htmlspecialchars() or anything similar.

 

Is it a stupid question to ask why that's not a security concern? Is it really that mysqli_real_escape_string() escapes HTML special characters as well?

 

Thanks in advance

[objnoob]

you use htmlspecialchars when echoing all user input to the browser. this ensures none of the user input is treated as HTML and prevents XSS ( cross-site scripting )

you use mysqli_real_escape_string() on all user input when you're using it as part of an SQL statment. this ensures none of the user input is treated as SQL and prevents SQL injection!

 

You do not have to call either for mysqli_error() since it is not user input!

 

 

 

Is it really that mysqli_real_escape_string() escapes HTML special characters as well?

No, mysqli_real_escape_string() does not convert HTML reserved characters to their entities.

[Ch0cu3r]

Why would you need to escape mysqli_error() for? It used to get the error message from MySQL when an error occurs.

 

Is it really that mysqli_real_escape_string() escapes HTML special characters as well?

No, that function only escapes harmful characters which could be used to make SQL injection attacks. An alternative safer way to use user input in queries is to use prepared queries

Edited November 1, 2013 by Ch0cu3r

[Mig21]

But it does have user input, for example if I change WHERE to WHERE1 this is the mysqli_error():

 

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'TimePeriod.timetableId = Timetable.id AND Timetable.id = '120'' at line 3"

 

120 came from the user! Escaped for SQL but not for HTML.

[vinny42]

You don't have to escape the error because you never display the error. The errormessage exists *only* for you, the programmer, so you can find out what went wrong. You never, ever, echo it to the browser. Never.

[objnoob]

You don't have to escape the error because you never display the error. The errormessage exists *only* for you, the programmer, so you can find out what went wrong. You never, ever, echo it to the browser. Never.

Good point!

 

 

 

 

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'TimePeriod.timetableId = Timetable.id AND Timetable.id = '120'' at line 3"

Yes, you're exactly right! Since it includes user input, it would be very wise to pass it through htmlspecialchars before outputting it to the browser. 

You should be validating too...  Check if 120 is a number, if not a number tell user there is an error! If number then construct SQL and execute.

Edited November 1, 2013 by objnoob

[Mig21]

Yeah, I thought so too, the problem is it seems that so far I'm the only one who's thought of it.

 

See for example the PHP manual where every example prints the return of mysqli_error(): http://us2.php.net/manual/en/mysqli.error.php

 

Or try to google mysqli_error escape or similar searches. You will find all over the pace the advice is entirely limited to escaping the SQL and sometimes escaping the HTML but never escaping the return of mysqli_error().

 

Which is why I'm so confused. Am I missing something? Should this be reported as a documentation error to php.net?

[Ch0cu3r]

Why are you so concerned with escaping mysql_error. It does not run any SQL code within the query, It only returns a snippet of the sql query where the error has occurred. As vinny mentioned earlier the only person that should see this error message is you the programmer.

 

If you have properly sanitized/validated the data before you start to use it within your SQL queries then no damage can be done, with the error that is returned.

 

When your site goes live on a production server no error messages from PHP/MySQL etc should ever be displayed to the user. Instead you'd log the error and maybe send an email to the site owner notifying them of what went wrong so they can fix it. The end user should see a plain simple error message informing them something went wrong. They shouldn't see the actual error message(s) from PHP/MySQL

Edited November 1, 2013 by Ch0cu3r

[DavidAM]

I agree that the error message should only be displayed in development. So escaping it should not strictly be necessary. Escaping it for mysql (mysql_real_escape_string) would not be the correct escaping for it in any case.

 

However, if the query contains HTML markup being written to the database, and the query failed, and you don't escape it using htmlspecialchars or htmlentities, then it could be difficult to read and interpret the error message. Again, however, this is information only for the developer and you can always see the raw output using the View Source feature of the browser.

 

If you setup so errors are displayed during development and logged during production and you use trigger_error to display/log the mysql error, you would not want to escape it when it is logged.

 

My suggestion would be, don't bother escaping it, at all. If, during development you get a displayed error that looks strange, use View Source to see it in the clear. If you do escape it, then you have to first check whether you are displaying or logging errors, so you can escape it or not. That would be a waste of programming time and execution resources.

 

As for the security risk of user injections; again, you are only displaying it during development; so unless you are testing XSS injection attacks, you are not likely to run into any security issues.

 

 

If you have properly sanitized/validated the data before you start to use it within your SQL queries then no damage can be done, with the error that is returned.

Not entirely true. The sanitation for the database is different from the sanitation for HTML display. Neither one provides protection for the other.

[vinny42]

 

However, if the query contains HTML markup being written to the database, and the query failed, and you don't escape it using htmlspecialchars or htmlentities, then it could be difficult to read and interpret the error message. A

 

If you are viewing the error in a browser, yes. But we all agree that this does not happen in a professional environment :-)

[mac_gyver]

lol, a popular forum software (SMF) had a XSS hole in the moderator backend when admins/mods viewed either reported posts or viewed the error history, i don't remember which, because content that came from a visitor was being displayed in a browser without being escaped.

 

if you are logging the mysqli_error() output and it can ever be viewed by anyone of importance, not just the visitor, in a browser, you need to escape render it harmless it at some point.

Edited November 1, 2013 by mac_gyver

[Mig21]

Well, sure there are reasons not to show the user errors coming from mysql queries, but how am I supposed to debug problems when they happen if the user can only tell me that "An error has occurred"? What do you guys do?

 

Do you log all the errors separately and review them periodically? Or give the user a ticket number that's associated with an SQL error that the user can't see but is stored some place else with the ticket number as a key for you to review?

 

Seems like a lot of work.

 

And actually, even in those cases - you have to review the errors and you're likely to read them in a browser or a mail client, both of which would be vulnerable to XSS. So even if I don't show the user the message, should I not always do the HTML escape on it?

[DavidAM]

It depends on the application and the audience.

 

In general, you log the error message, and any pertinent information (such as date-time, page, user input, etc) that might help you determine the cause and fix the problem. You review the log periodically. Since the log is a file, if you are reviewing it in a text editor, you do NOT want the HTML escaped -- you want to see the raw data.

 

If you write a browser-based front-end to review the log, you would escape the data AS YOU WRITE IT TO THE HTML PAGE (NOT THE LOG). This way, you will SEE what the user entered NOT the escaped encoding.

 

If you are talking about an in-house application where the users might actually call you while the message is displayed, you MIGHT choose to display the error. IF you display it, you WOULD escape it WHEN YOU WRITE IT TO THE PAGE. However, even in this case, I would write it to a log and display a "There was a problem" message. It is my experience that users do NOT read error messages anyway, even when they immediately call me, the answer is always "I didn't read it". Write it to a log, and when they call you, you can open the log to see the message. Truth be told, you WANT to see the error message yourself; all users interpret or paraphrase messages when they read them, or skip words, or read them wrong, or make them up so they don't look dumb for not having read it.

[vinny42]

 

Do you log all the errors separately and review them periodically?

 

You log everything. e-ve-ry-thing. We're talking errors here, unexpected behaviour, so the more information you have, the more likely it is that you can solve the problem.

 

 

Seems like a lot of work.

 

And that's why we don't do that :-)

 

The only thing users need to know is "do I call IT for this, or did I do something wrong myself?" A nice clear and short errormessage is often helpfull, if you are nice to your users they will look at the message and remember it, but anything beyond five words is simply forgotten if it's not clearly a user-error.

 

 

And actually, even in those cases - you have to review the errors and you're likely to read them in a browser or a mail client, both of which would be vulnerable to XSS. So even if I don't show the user the message, should I not always do the HTML escape on it?

 

I don't send logs as HTML emails and I don't think mailclients even do scripting in the first place (?), and I view the logs through SSH. If you do want to view it in HTML then I +1 on DavidAM; escape before you print, but log the original.

[Mig21]

Yeah, I guess you're right, so I'll get rid of these messages after the release candidates and maybe one day I'll have the patience to write an error logger and reviewer.

 

I'll still try to add a note to php.net for those who didn't think it through, that warning really should be there since it's not immediately obvious (like most XSS issues!).

 

Thanks everyone!

[vinny42]

 

 so I'll get rid of these messages after the release candidates and maybe one day I'll have the patience to write an error logger and reviewer.

 

Removing messages before going live is a bad idea, experience shows that you never remove all the messages and once you have removed them, you have no way of debugging them because you have no messages.

 

Write a prioper logger *now* and save yourself a lot of problems.

 

It should not be much work anyway because if you have designed your code properly you should have a single function or class that executes queries and that's the only place where the logging needs to be implemented.

 

Even if you have spammed your code with mysqli_* statements you can just rename them to a custom function that does the logging and otherwise behaves like the mysqli_* functions.