[ postfixadmin-Bugs-1779823 ] function escape_string() incorrectly handles no magic quotes

[SourceForge.net <no...@so...>]

Bugs item #1779823, was opened at 2007-08-23 00:23
Message generated for change (Comment added) made by christian_boltz
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=937964&aid=1779823&group_id=191583

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Core
Group: None
>Status: Closed
>Resolution: Fixed
Priority: 5
Private: No
Submitted By: FredKilbourn (fredkilbourn)
Assigned to: Nobody/Anonymous (nobody)
Summary: function escape_string() incorrectly handles no magic quotes

Initial Comment:
When magic quotes is turned off in php, unable to enter any data into postfixadmin and php throws errors in the error_log.

Attached is a patch to fix with more detail on problem and solution.

----------------------------------------------------------------------

>Comment By: Christian Boltz (christian_boltz)
Date: 2007-10-07 21:31

Message:
Logged In: YES 
user_id=593261
Originator: NO

This is fixed in the latest SVN version. escape_string was rewritten and
now always uses the db-specific escape functions.

BTW: I didn't check the whole code, but I'm quite sure that this function
is only used to escape strings for SQL queries. Everything else usually
needs htmlentities() ;-)

----------------------------------------------------------------------

Comment By: FredKilbourn (fredkilbourn)
Date: 2007-09-06 18:51

Message:
Logged In: YES 
user_id=1873536
Originator: YES

nobody @ 2007-09-06:
  What you say is actually the correct fix when magic quotes is off, but
it does not address the base problem(s) here:

Problem:
1 The escape_string() function calls these specific database escape
functions only if magic_quotes_gpc is off.
2 This is the only place these specific database escape functions are
called anywhere in the codebase.
3 When magic_quotes is on, all these database escape functions are
skipped
4 When magic_quotes_gpc is on, it is the equivalent of PHP function
addslashes() on any get/post/cookie data.
5 PHP function addslashes() is strictly inappropriate for escaping
database query variables

Therefore:
1 When magic_quotes_gpc is on, no database queries are properly escaped
2 With my attached patch, no database queries are properly escaped if
magic_quotes_gpc is on or off

The even greater issue is this:
- In what cases is escape_slashes being called to escape a db query, or to
escape for another purpose?
- I don't have time to go through the entire codebase and analyze this
question but the more appropriate escaping implementation is to have:
--- escape_string_db - for escaping db query vars only
--- escape_string - for escaping other vars (if necessary - i didnt go
through the code base so i dont know for sure)

escape_string_db pseudocode:
if( magic_quotes_gpc == on )  //if magic quotes is on, stripslashes to
undo addslashes that was auto-done
  stripslashes( $var )
return db_specific_escape( $var )    //use appropriate db specific escape
function


escape_string pseudocode:
if( magic_quotes_gpc == off ) //if magic gpc is off use addslashes to
emulate functionality
  return addslashes( $var )

----------------------------------------------------------------------

Comment By: Nobody/Anonymous (nobody)
Date: 2007-09-06 12:34

Message:
Logged In: NO 

The problem is that db_query function connects to the database, run the
query, and close db connection.

escape_string() calls mysql_real_escape_string($string).
when no dblink is given to mysql_real_escape_string() as 2nd parameter, it
takes the last connection opened by mysql_connect. If it cannot find any
connection, the function tries to connect with default mysql connection
parameters (not set for me).

The workaround I found is to comment mysql_close() in the fonction
db_query() in the functions.inc.php

----------------------------------------------------------------------

Comment By: yves teixeira (nictuku)
Date: 2007-08-30 07:15

Message:
Logged In: YES 
user_id=572172
Originator: NO

Thanks, Fred. It works for me.

You saved the day. :-)

Yves (rimuhosting.com)

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=937964&aid=1779823&group_id=191583