Forgot your password?
typodupeerror
PHP Programming Security

Serious Crypto Bug Found In PHP 5.3.7 165

Posted by timothy
from the pronounced-pfffffpp dept.
Trailrunner7 writes "The maintainers of the PHP scripting language are warning users about a serious crypto problem in the latest release and advising them not to upgrade to PHP 5.3.7 until the bug is resolved. PHP 5.3.7 was just released last week and that version contained fixes for a slew of security vulnerabilities. But now a serious flaw has been found in that new release that is related to the way that one of the cryptographic functions handles inputs. In some cases, when the crypt() function is called using MD5 salts, the function will return only the salt value."
This discussion has been archived. No new comments can be posted.

Serious Crypto Bug Found In PHP 5.3.7

Comments Filter:
  • by Niac (2101) on Monday August 22, 2011 @07:35PM (#37173218) Homepage

    Who cares about testing security code for regressions?

    I'm seriously astounded that the php development community doesn't have acceptance testing around this sort of thing. In this day and age, why on earth is it the case that bugs like this get through?

    • by PCM2 (4486) on Monday August 22, 2011 @07:37PM (#37173234) Homepage

      I'm seriously astounded that the php development community doesn't have acceptance testing around this sort of thing. In this day and age, why on earth is it the case that bugs like this get through?

      Speaking as an occasional PHP developer, you must be new here.

      • by Niac (2101)

        Okay, fair point. I try to give PHP the benefit of the doubt... ;) I'd like to think that they've got automated testing of submitted code. I'd like to... :-(

        • Re: (Score:3, Informative)

          by Rob Aley (2438044)
          While it could have been caught, the bug was actually a result of testing, namely the use of the Coverity static analyzer which flagged up "strcat(passwd, "$");". In this particular case it was safe use strcat, but was erroneously changed to "strlcat(passwd, "$", 1);" to avoid the warning. Its a lesson perhaps that automated tests aren't the silver bullet for avoiding bugs, understanding the code itself is just as important. It also raises the question of whether much of the criticism in the discussions b
          • by gweihir (88907) on Tuesday August 23, 2011 @07:55AM (#37176524)

            Automated tests are nice, but fixing the code requires somebody that _really_ understand what they are doing. Fixing problems found in automated tests so that the tests stay quiet without understanding what is going on is about the most stupid thing you can do in security-relevant code. It is grounds for immediate and permanent removal of code maintainer status.

            Also note that the Debian OpenSSL disaster was a result from doing the same thing with valgrind, so there has been at least one very public warning about this sort of thing.

            • by X0563511 (793323)

              Are you certain that it was indeed someone not knowing wtf they were doing? It could (though I do realize this is php..) - COULD have been a mistake by an otherwise knowledgeable individual.

              • by gweihir (88907)

                The person responsible did rather obviously not understand the limits of his own understanding. In normal bugs a more experimental approach to fixing code is fine. But with crypto, you have to understand what you are doing, as crypto very easily breaks in ways that are non-obvious and leave the software functional, yet insecure. Finding this out does not even need in-dept crypto knowledge. Almost all major attacks on crypto in the last few years have been on the implementation, not the crypto itself.

                So, yes

          • by Gunstick (312804)

            a good read on how it happened
            http://pwnhome.wordpress.com/2011/08/22/php-crypt-bug/ [wordpress.com]

      • by rainmayun (842754) on Monday August 22, 2011 @07:46PM (#37173294)
        True, PHP has a history of "winging it", but by now they should be doing a pretty damn extensive suite of regression tests against each release candidate, if not each build. At this point in its life and supposed maturity, the PHP Group should really be doing better.
        • by Anonymous Coward

          PHP has gotten itself into a vicious cycle where it inherently can't get better.

          Anyone who knows what their doing will refuse to use PHP. That means that only the worst "programmers" out there will even consider it, let alone use it.

          WIthout having good developers using it, it'll never have good developers contributing to it. No good developer would want to publicly admit that they've contributed to PHP.

          At this point, some fool will throw out some crap like, "OmG but W1kip3D1A n faceb00K yooze PhP!@!#%@!!".

          • by rainmayun (842754)
            A bug in a library function shows how a language is poorly designed? Methinks you need a little more logical organization to your thoughts. and I can't help but laugh at "no good developer would want to publicly admit that they've contributed to PHP". Perhaps no good developer would want to admit to posting your comment, hence Anonymous Coward status.
            • by EvanED (569694) <evaned.gmail@com> on Monday August 22, 2011 @11:01PM (#37174390)

              I think that the post you replied to was a bit extreme, but it's not the bug in the library function that caused him to say that: it's the fact that the PHP project lacks the testing infrastructure that any reasonable project of that size would have.

              Anyone can commit a bug; that's easy and excusable. What makes it look like PHP is developed by a bunch of 12 year olds is the fact that they have a test suite with a test that exhibited the bug, and yet no one ran it before they made a release, because they've got too many failing tests so it just got swamped in with that noise.

              I'm working on some dinky pieces of research software, and while we probably don't have as extensive a test suite as PHP does, we have a way better testing regimen. A project like PHP should have a CI server that runs their tests at least nightly, and a release shouldn't be made while there are failing tests. That's what expected failures are for. (They even know about expected failures, but still have over 200 failing tests for some reason.) Even we've got that.

              It's the QA that's messed up, not the coding.

              • by mwvdlee (775178)

                Yup. The problem here seems to be with procedures rather than technology. Bugs happen. What matters is what you do when a bug happens. The bug was reported before release date and the bug should have been marked as a showstopper.
                PHP 5.3.7 should not have been released with a known bug of such magnitude. It's an open source project without commercial interrests; it doesn't matter if a release is delayerd for a couple of days.

            • by Firehed (942385)

              No, it actually was a bug in the way the function was called, not the library itself - see comment number four: https://plus.google.com/113641248237520845183/posts/g68d9RvRA1i [google.com]

              In reality, almost nobody* is going to call md5 via crypt() when a standalone md5() function exists, and people are often slow to deploy new versions of PHP - especially major shared hosting providers. Those who manage their own PHP installs and deploy shortly after release tend to have their own set of unit tests, which very likely wo

              • by makomk (752139)

                In reality, almost nobody* is going to call md5 via crypt() when a standalone md5() function exists

                Yes they are, because the two don't do the same thing. md5 just calculates the md5 of data, whereas the md5 support of crypt uses salting and a large number of rounds to make password cracking harder.

            • by Haeleth (414428)

              A bug in a library function shows how a language is poorly designed?

              No, but releasing an update without even running the existing unit tests shows how amateurish the whole PHP project is. The terrible design of the language is also a reflection of its amateurish nature.

              It's true that some websites manage to do wonderful things with PHP, but then it's also true that some artists manage to make wonderful sculptures out of manure. That doesn't mean it's a good choice for most people.

          • by Joce640k (829181)

            None of that changes the fact that it's a horribly "designed" language

            It was "designed"??? Whoa...I think I just experienced a negative reality inversion.

    • by thue (121682) on Monday August 22, 2011 @07:55PM (#37173332) Homepage

      From the Bug report [php.net]:

      > Confirming, some very recent update broke it - right now unit tests fail on SVN. I wonder if nobody run it before release?

      So they do have a unit test for that. They just didn't run it before release :).

    • Re: (Score:3, Informative)

      by msauve (701917)
      "I'm seriously astounded that the php development community doesn't have acceptance testing around this sort of thing."

      Two things.

      1) The problem was found and announced by "the php development community," and presumably found by them, too (admittedly, not prior to release).
      2) Why aren't you involved in acceptance testing, if you see a problem with how it's being done?
      • by Korin43 (881732)

        2) Because it's PHP. No one wants to be involved with that.

      • by Chuck Chunder (21021) on Monday August 22, 2011 @10:59PM (#37174378) Homepage Journal

        1) The problem was found and announced by "the php development community," and presumably found by them, too (admittedly, not prior to release).

        That seems entirely incorrect. According to the bug report [php.net] it seems to have been found by someone external, it was found in a release candidate not the released version and seemingly filed before the release was made.

        2) Why aren't you involved in acceptance testing, if you see a problem with how it's being done?

        Speaking for me, we pay Zend for server licences and imagine that in someway contributes to a professionally run project. Though I have to say we are becoming increasingly unsure as to whether we get any value for money for that, of the security fixes that 5.3.7 fixed I haven't noticed any of them being pushed to Zend Server in a priority fashion and I don't think we've ever had a single support question resolved satisfactorily. Sometimes being a Zend customer seems merely to open you up to sales pushes.

      • by pclminion (145572)

        Why aren't you involved in acceptance testing, if you see a problem with how it's being done?

        I am involved. It's open, right? So, I hereby declare myself to be involved. Further, I declare this release to be unacceptable. I do not certify it, and the release is hereby invalidated. That was easy!

    • by AvitarX (172628)

      I know, that's the type of thing we expect from debian when working with cryptography.

      • Few things you are failing in here. First, "Debian" is down to one guy when it comes to packaging PHP. More or less, Ondej Surý does it all, since last year, Raphael Geissert started being busy. I myself work more on packaging PEAR packages, and I have enough work. So if you want things to become better for the packaging of PHP in Debian, then you need to contribute! Create a new user on Alioth, register the project and the pkg-php-maint list, and start helping. Because for sure, we don't have, in Debi
        • Not to dismiss the serious amount of time and effort you, Ondej, Raphael and all packagers put in, but I'm pretty sure the parent was making a joking reference to the OpenSSL entropy bug.

  • Rut Roh raggy!
    • by Anonymous Coward

      From that wikipedia article "The presented attack does not yet threaten practical applications of MD5".

      A collision attack allows you find pairs of values that have the same MD5 hash, but these pairs are very rare.

      This is a problem for digital signatures, since an attacker could create two files (one malicious one not) that both have the same signature. It's not a problem for passwords, since attackers can't control the password you pick.

      • I've read that last paragraph 6 times and still don't understand what you are getting at. The attacker can't control what message you create any more than the password, so what is the difference?
        • by mgiuca (1040724)

          Read up on the difference between Collision attack [wikipedia.org] and Preimage attack [wikipedia.org].

          MD5 is vulnerable to collision, but not yet preimage attacks. The preimage attack the GP is mentioning is something like this: Alice is required to digitally sign off on all money withdrawals from an account, and MD5 is used as a hash algorithm. Bob creates two documents, one saying "I authorize the withdrawal of $100." and another saying "I authorize the withdrawal of $1,000,000." He uses a collision attack to ensure that these two docu

          • Ah, so the only person that can really take advantage of it would be the person creating the legitimate one as well. It kind of sounded from the original that the attack could be used to impersonate someone. Thanks for clarifying.
            • by mgiuca (1040724)

              Hmm, when you say "take advantage of it" are you talking about the well-known MD5 collision attack, or the PHP crypt bug?

              Re the collision attack: I think you are right. But still, advice going around is "don't use MD5". Researchers are still chipping away at it, so there's no telling when they'll have a working preimage attack.

              Re the PHP crypt bug: my understanding is that it's quite serious in that it can throw away the input and just use the salt, causing widespread MD5 collisions. So that could definitel

    • The MD5 collision vulnerability only allows you to generate pairs of plaintext that share the same hash. It does not allow you to find a colliding plaintext-B from a given plaintext-A. It also does not allow you to compute a plaintext from a given hash.

      In terms of passwords, here's what the exploit looks like: The attacker generates a pair of colliding texts; they use one of them as a password; the other text can also be used for the password. There are some contrived scenarios where this might be a pro

    • Does salting garbage result in something edible?

      Well, yes?
      http://en.wikipedia.org/wiki/Salt_(cryptography) [wikipedia.org]

      Perhaps put very simply.. say you find a file full of e-mail addresses followed by md5 codes, e.g.
      example@example.com 5eb63bbbe01eeed093cb22bb8f5acdc3

      You deduce that they may be hashed login codes, so you run your MD5 hacking tool and find the password to be either "hello world" or "flurblgrabl".
      You enter either "hello world" or "flurblgrabl", and you're in.

      Now let's say you find instead:
      example@example

      • by c0lo (1497653)

        Does salting garbage result in something edible?

        Well, yes?

        The problem with salting: transfer the matter into "security by obscurity".

        If the repo of you passwords leaks, one can assume the salt grains would leak too. Then you are not better than having the hashed password alone to attack.

        • What's fixed by using salt in your passwords is that the leaked password file can't be compared against a precomputed password dictionary.

          • It still can, salt just makes it impractical. Of course if you had a true Turing machine even salt would be useless, but alas nobody as of yet has come up with an infinite storage device.
        • The problem with salting: transfer the matter into "security by obscurity".

          Which is what passwords are in the first place, and, by extension, any mechanism of manipulating or digesting them.

          If the repo of you passwords leaks, one can assume the salt grains would leak too. Then you are not better than having the hashed password alone to attack.

          Password hashes are typically stored in a database. The salt is typically part of the configuration store. Most systems (Wordpress being the singular counterexample I can think of) store user data separate from configuration data, and configuration data is usually left flat files. (LDAP is an alternative, but I don't believe most services use it)

          So even if the password hashes leak, it's unlikel

          • by Goaway (82658)

            Typically, salt and hash are stored together as one string, and each password will have its own randomly selected salt.

            Salting is for defending against precomputed dictionary attacks. You're not supposed to keep the salt secret.

            • That's not how I've generally seen salts used. Generally, I've seen salts used like "echo $SALT$DATA|md5sum".

              I'm not disputing that the method you describe isn't done, I just haven't seen it done that way.

        • by Firehed (942385)

          False.

          Unless there happens to be a pre-computed rainbow table that includes the salt of the password (likelihood: practically zero), you are still massively better off. In that case, you'll have to generate your own rainbow tables if there's a single salt for all entries, or go cry yourself into a corner in the hopefully more likely scenario where each entry has its own unique salt (because you now need to bang out a rainbow table for every entry in the users table).

          Nearly all security is based on obscurity

        • The problem with salting: transfer the matter into "security by obscurity".

          The only point of per-site (and ideally per-user) salt is so that a pre-computed rainbow table attack does not work. And salt is a semi-public piece of information. It has to be published in enough places through your code that you must assume that the attacker knows how you salt. And often, the salt is stored right along side the password in the password hash.

          Without salt: attackers create the huge rainbow table of about a
      • Run an attack against that, take the results (potential salts), now run an attack against all the other MD5 signatures with the found potential salts tacked on until you get matching MD5s.
        Now the correct salt has been identified

        If you know what a salt is, you should also know that using the same salt for different accounts is very very bad.

        • If you know what a salt is, you should also know that using the same salt for different accounts is very very bad.

          Yep - but sadly commonplace.

        • by gorzek (647352)

          Indeed. I remember running some WWWBoards back in the '90s, and even those salted the password. I believe they just grabbed a particular substring of each password and fed it as the salt into the crypt function, so you had a (more or less) unique salt for every password. And that was a simple perl script from the '90s. I would hope everyone is doing at least a little more than that today.

    • MD5 should be deprecated, but the collision attack only invalidates signatures; it doesn't help you extract a password from its hash.

      Currently there is no feasible non-dictionary attack for that (the preimage attack found in 2009 still has complexity >2^120), and the dictionary attacks are defeated by salt. So in this narrow context, yes.

      (Of course, this would end if a somewhat more efficient preimage attack is found. 2^120 is orders of magnitude beyond usefulness, but not many orders...)

      • by c0lo (1497653)

        MD5 should be deprecated, but the collision attack only invalidates signatures;

        Because you are not "decrypting" the password, finding a collision will be an attack for passwords as well - fortunately, as others pointed out, it's currently still 2^120.

        and the dictionary attacks are defeated by salt

        I argue that using a "salting for passwords" it's useless (doesn't do harm, but doesn't bring in too much good either) . Unlike signatures/message digests, the checksum is not made public (the passwords - or their checksum - are stored in a "secure place") and the attacker usually has longer time for cracking a password - until you chang

        • by norpy (1277318)

          You should be salting each password with a unique salt - and storing the salt with the hash!

          The reason for salting the password is to invalidate rainbow tables by effectively making the hashing function unique for each password.

          Yes it's a problem for an attacker to get your hashes, but if they do you have made sure they have to break each and every password separately rather than the whole lot at once. Or even worse having precomputed MD5 tables being able to break them immediately.

        • by ais523 (1172701)

          What you're missing is that even if they have both the hash/checksum and the salt, they then have to start bruteforcing the password from scratch. Because they didn't know the salt in advance, they couldn't have precomputed a dictionary of hash -> password mappings like you can with unsalted passwords, so each password has to be bruteforced separately. If the passwords aren't salted, and are just plain md5 (say), it's entirely possible that they already know what the md5 reverse-maps to because they've s

        • by cduffy (652)

          Of course an attacker who steals your password database knows the salt values in use for each account.

          But unless they have a rainbow table calculated with every possible salt value taken into account, they can't do a simple lookup on a precomputed map of hash values to top-5,000,000-common-passwords and hope to get a match... so their expense to actually break any of those passwords is much, much higher.

    • by gweihir (88907)

      Collision attacks are no threat to passwords. They are a threat to things like certificate signatures when the certificate is made by the attacker.

      Please do not post nonsense.

  • by Anonymous Coward

    This bug has been fixed already. See https://bugs.php.net/bug.php?id=55439 [php.net]
    Main problem was if an aplication stores its hashes in a database and use them as authentication then:
    $valid = crypt($pw, $crypt);
    will always be TRUE regardless of $pw
    For all this, PHP Team said it is fixed in SVN and recomending to wait (upgrade) till 5.3.8

    • by nedlohs (1335013) on Monday August 22, 2011 @08:28PM (#37173514)

      Nope.

      $valid will be the return value of crypt which will be true in the non-broken code as well.

      $crypted == crypt($pw, $salt)

      will always be true in the broken code, if $crypted was created with any old password and the same salt.

      Of course if you have existing such password, they'll always match false, so no one is going to be able to change their password and trigger the problem anyway :)

  • by Anonymous Coward

    .. if the code never calls the crypto. That is the case here.

    • by gweihir (88907)

      It is a crypto protocol implementation bug, or crypto usage bug, short "crypto bug". Actual bugs in crypto algorithm implementation are so rare it does not make sense to reserve the term for them.

  • Poor QA (Score:4, Informative)

    by TheNinjaroach (878876) on Monday August 22, 2011 @08:17PM (#37173448)
    The PHP project has shown some pretty poor QA when it comes to defects in their code.

    Hell, their ODBC interface has been returning wrongly typed data [php.net] for years now and nobody on the project seems to care. It's not like ODBC is something brand new and still widely misunderstood.

    It's almost like the people who build PHP aren't even interested in maintaining it.
    • by Xest (935314)

      "It's almost like the people who build PHP aren't even interested in maintaining it."

      It's not really that, it's just the PHP ethos, PHP has never really been about good software development and that flows not just through the language itself but development of the language. It's not that PHP is inherently bad- rather than focus on good practice, it focuses on just getting things done, and this kind of works, but it also means that you'll more frequently see things like this when you throw best practice out

  • by GPLHost-Thomas (1330431) on Monday August 22, 2011 @08:47PM (#37173628)
    The internal crypt() function of PHP is only there whenever the system function doesn't exist. So for example, in Debian, only the blowfish encryption is affected, all other encryption are using the system. Here's Ondrej post about it:

    http://lists.alioth.debian.org/pipermail/pkg-php-maint/2011-August/009328.html [debian.org]

    I am guessing that this will be the case in most Unix distribution, but it will be an issue on platforms like Windows. So, maybe this is just too much buzz...
    • by EvanED (569694)

      The internal crypt() function of PHP is only there whenever the system function doesn't exist.

      This is not correct.

      Ondrej's post you link to is specifically referring to the patched version of PHP that you get from the Debian repository. One of the patches Ondrej applies makes PHP use the system crypt(). Without that patch -- with the stock PHP code -- PHP uses its own crypt(). Now, other distributions might apply Ondrej's patch, but I certainly wouldn't count on it, and you definitely will have a broken cry

    • Isnt running php on Windows the equivalent of hanging a big sign on your back that says "hack me"?
  • I've seen a few people here note the fact that "salts" are used to add complexity to a password where no complexity exists. I believe this is incorrect, or at the very least not entirely the truth.

    Many are assuming that the salt is something that wouldn't be compromised, i.e.

    $SITE_SALT = 'LULZYOUCANTGUESSTHIS';
    $HASH = MD5($SITE_SALT + $PASSWORD);

    I think this is a bit of a misunderstanding. The salt is not necessarily intended to be a secret value unless you can ensure the security of that value, that is the
  • Everyone should be using mysql_real_crypt by now.
    • by RobNich (85522)

      I thiiiink you're joking? Seriously, though, I've been using mcrypt in PHP for years, even though it's STILL! not even available through yum for CentOS. libmcrypt and mhash have been required by my infrastructure for a long time, and I've never bothered to use the built-in crypt(). I'm a little peeved that there was no unit test that caught this bug, but I'm glad to be unaffected.

  • I wonder if PHP has the same problem we do in Hadoop-land... the lack of enough qualified security people interested enough in a project to actually review code. For example, I'd love for someone with a clue to review Alfredo ( http://cloudera.github.com/alfredo/docs/latest/index.html [github.com] ) before we build a dependency on it ( https://issues.apache.org/jira/browse/HADOOP-7119 [apache.org] ) . But it seems as though getting the right people involved is extremely difficult. :(
  • Worst of all worlds (Score:4, Interesting)

    by 93 Escort Wagon (326346) on Tuesday August 23, 2011 @02:25AM (#37175244)

    I know there's no reason a skilled programmer can't use php, but in my experience the users that request php access are generally the users who you'd least want to have any sort of script-level access to your servers. When I've explained to requestors why we don't generally provide php, I've been told on several occasions "I don't want or need the ability to run scripts! I just want to create php web pages." Oh, and mysql access requests usually come hand-in-hand with php requests.

    I remember one guy, quite a few years ago, who asked us to 1) enable php on our department's web server; and 2) give him access to create and run php scripts. To demonstrate to us that he wasn't just another newbie... he wrote a php script and placed it on his own personal box as a demo of his coding skill. This script let anyone, anywhere, examine the content of any file in the /etc/ directory via an easy to use web interface.

    We politely declined his request.

    • > We politely declined his request.

      If this predated /etc/shadow, I would have just mailed him back his root password. :)

It's a poor workman who blames his tools.

Working...