WordPress.org

Make WordPress Core

Opened 9 years ago

Closed 7 years ago

Last modified 7 years ago

#3286 closed defect (bug) (duplicate)

Handling of escape sequences is muddled and non-compatible

Reported by: cdavies Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.0.4
Component: Security Keywords:
Focuses: Cc:

Description

Wordpress should use SQL-style escape sequences in its SQL statements, and HTML style escape sequences in its output to the browser. Instead, it uses C-style escape sequences in its SQL.

This causes Wordpress not to function correctly with MySQL in NO_BACKSLASH_ESCAPES mode, and makes porting to other DBMS such as SQLite difficult.

The fix for this problem is to remove all instances of addslashes(...) from the code, and rewrite the escape function in wp-db.php.

While I was checking this defect was not a duplicate, I also noticed a security defect reported against an ancient version of wordpress took issue with the way SQL was escaped, the fix for that appears to have regressed.

Change History (9)

comment:1 @ryan9 years ago

We tried mysql_real_escape_string() for awhile but it caused a lot of character set related problems. Someone needs to figure out how to make it play friendly.

comment:2 @cdavies9 years ago

mysql_real_escape_string is never going to play nice, it is fundamentally designed with ASCII in mind, and won't deal with multibyte strings at all. I'm really, really suprised the current system hasn't garnered you any complaints from users who need multibyte strings to be honest.

In reality, you should just be following SQL standards and only escaping single quotes () in your SQL statements. This may require you to demand a later version of MySQL, but being honest, who actually uses MySQL v3 anymore anyway?

comment:3 @ryan8 years ago

So escape only single quotes and escape them in the fashion. Also, make sure $wpdb->escape() is used only when escaping on the way to the DB, and use addslashes() when escaping for things like HTML and JS.

comment:4 @ryan8 years ago

Oops, trac converts doubled-up single quotes to italics. :-)

comment:5 @rob1n8 years ago

I believe we now require MySQL 4.0 or greater.

comment:6 @Nazgul8 years ago

  • Milestone set to 2.4 (future)
  • Priority changed from high to normal
  • Severity changed from major to normal

comment:7 @pishmishy7 years ago

See also #5455. Tempted to close this as a duplicate of that, or t'other way around.

comment:8 @thee177 years ago

  • Resolution set to duplicate
  • Status changed from new to closed

comment:9 @thee177 years ago

  • Milestone 2.5 deleted
Note: See TracTickets for help on using tickets.