WordPress.org

Ready to get started?Download WordPress

Forums

SQL injection bug report. (11 posts)

  1. dsadinoff
    Member
    Posted 9 years ago #

    WordPress 1.5 has an SQL-injection vulnerability. Sorry if this is the wrong forum for this sort of thing, but there doesn't seem to be a place that says "report bugs here".

    Anyway, on the change-password screen (wp-admin/profile.php), the code looks like so:

    $updatepassword = "user_pass=MD5('$newuser_pass'), ";
    ...
    $wpdb->query("UPDATE $wpdb->users SET user_firstname='$newuser_firstname', $updatepassword ...

    so for some reason the author of this code decided to defer the MD5 operation to the RDBMS rather than doing it in memory. Anyway, this piece of fancy interpolation is buggy, because the password is not guaranteed to be quote-free.

    Putting a quotation mark in the new password breaks the page, ("your session has expired"), but more importantly opens up the entire database to sql injection attack.

    http://www.unixwiz.net/techtips/sql-injection.html

    cheers.

  2. vkaryl
    Member
    Posted 9 years ago #

    http://trac.wordpress.org/ - but be sure you dig through the "solved" stuff, because the version is up to 1.5.1.3, and while I can't be sure, I think what you're talking about here is a done deal....

  3. skippy
    Member
    Posted 9 years ago #

    This is partially mitigated by the fact that the attacker needs to have an account on your blog. So the first step to stay safe is to make sure you don't let strangers register for accounts.

    I'm looking into this further.

  4. dsadinoff
    Member
    Posted 9 years ago #

    Sounds like the bug found it's way to the right hands. Would one of you be so kind as to open a ticket for me?

  5. Michael Adams (mdawaffe)
    Member
    Posted 9 years ago #

  6. Mike Little
    Member
    Posted 9 years ago #

    In fact, the input is properly escaped. There is no injection vulnerability that I can see in the current version (1.5.1.3)

  7. dsadinoff
    Member
    Posted 9 years ago #

    MikeLittle, I'm no PHP-guru, just a humble perl guy since 1992. I can't see where there is any escaping happening here.

    http://trac.wordpress.org/file/tags/1.5.1.3/wp-admin/profile.php

    Looks to me like the dataflow is:

    $pass1 = $_POST["pass1"];
    do_action('check_passwords', array($user_login, &$pass1, &$pass2));
    $newuser_pass = $pass1;
    $updatepassword = "user_pass=MD5('$newuser_pass'), ";

    Can you explain where the escaping is? What does do_action('check_passwords'..) do? It looks like some sort of plug-in hook. Is there a standard plugin that does the esacaping?

  8. I'm no PHP guru either, but it would seem to me that do_action('check_passwords'..) does an action which checks (confirms?) passwords, or whatever action the core files define as check_passwords .

  9. dsadinoff
    Member
    Posted 9 years ago #

    I have yet to see a line of code associated with check_passwords. I don't think it does anything.

  10. skippy
    Member
    Posted 9 years ago #

    check_passwords is a plugin hook. You can write plugins to check passwords, allowing you to enforce arbitrary password requirements on a site-by-site basis.

    Line 136 of wp-settings.php shows:
    if ( !get_magic_quotes_gpc() ) {
    $_GET = add_magic_quotes($_GET );
    $_POST = add_magic_quotes($_POST );
    $_COOKIE = add_magic_quotes($_COOKIE);
    $_SERVER = add_magic_quotes($_SERVER);
    }

    So regardless of the server's magic quotes setting, GET and POST data should be properly escaped. The function add_magic_quotes() is a wrapper for addslashes() defined at line 1818 of wp-includes/functions.php.

    The first thing profile.php does is require admin.php. The first thing admin.php does is require wp-config.php. The last thing wp-config.php does is require wp-settings.php.

    So by the time profile.php grabs the query string, add_magic_quotes() should have properly sanitized it.

  11. dsadinoff
    Member
    Posted 9 years ago #

    But why is adding slashes a good thing? isn't it a little early for that? Meanwhile, this slashify_gpc business seems to make a mess of things, where there are rules, exceptions to rules, and even more rules, so the security interface is hard-to-read and understand. Is it possible to write a plugin which will cause the escaping to fail?

    Anyway I agree with you, I am mistaken in saying that there is a security hole in wordpress. Rather, there is a bug where no password can work which contains characters escaped by the addslashes() function. It looks like the correct hash is getting written to the database, but the cookie seems to be wrong. Perhaps it's this slashify_gpc thing. I'm not sure. I've tested this, and I can't get a password with a slash or a single-quote to work properly.

    At the very least, consider the wp_setcookie() at line 70. It's passing the slashified password into the cookie routines. That doesn't sound right.

    Again, my apologies for the false alarm. When the login system failed upon setting the password to something containing a quote, I assumed the worst.

    Meanwhile, shouldn't wordpress start using the much-simpler PHP5 auto-escaping systems? http://www.zend.com/php5/articles/php5-mysqli.php#Heading11

Topic Closed

This topic has been closed to new replies.

About this Topic