[wp-trac] Re: [WordPress Trac] #3036: theme-editor.php broken: stripslashes() and add_magic_quotes() screw up CR LFs

[WordPress Trac]

#3036: theme-editor.php broken: stripslashes() and add_magic_quotes() screw up CR
LFs
------------------------------+---------------------------------------------
 Reporter:  astounding        |        Owner:  anonymous
     Type:  defect            |       Status:  closed   
 Priority:  normal            |    Milestone:           
Component:  Administration    |      Version:  2.0.4    
 Severity:  major             |   Resolution:  fixed    
 Keywords:  theme-editor.php  |  
------------------------------+---------------------------------------------
Changes (by astounding):

  * resolution:  => fixed
  * status:  new => closed

Comment:

 Resolved!

 Further tracking shows that add_magic_quotes() calls the database's
 quote() which in my case is PHP's mysqli's escape function.  And of course
 the database's escape function will convert CRs and LFs to the "\r" and
 "\n" sequences.

 If the data's being pre-quoted to be safe for database insertion, why
 later strip slashes?  That's brain-dead poor design.  Better to keep the
 data "tainted" and untrusted in the original user-supplied form up until
 the time it actually needs to be inserted, and then quote it there.  As
 long as all modules and components match this behavior (and consider all
 user-supplied data tainted), you still get security, without weird
 oddities that almost always creep in when

 Oh, and NOT using the database's quoting mechanism (as in the current
 mysql db module where the database's escape functions are commented out)
 does open up the threat that in some future database upgrade, the non-
 database-provided escaping function will fail to escape something
 critical, opening one up to SQL injection vulnerabilities.

 Since the database-provide escaping function does things that other
 sections of code and/or modules do NOT desire (i.e. in this case
 converting CRs and LFs), some well-meaning WordPress coder, rather than
 undertaking the arduous and immense task of fixing a fundamental design
 problem (And I agree, I'm not going to undertake that task either!--WAAAAY
 to big a job for the little time available...) the coder just commented
 out the database escaping code and substituted PHP's addslashes() (which
 is NOT guaranteed to always be free of potential SQL-injection
 vulnerabilities, esp. as new database types are added in the future).

-- 
Ticket URL: <http://trac.wordpress.org/ticket/3036>
WordPress Trac <http://wordpress.org/>
WordPress blogging software