Some closure on the 1.6.4 Security Vulnerability

A little over two weeks ago we announced the discovery of a rather significant vulnerability which may have effected some users. At the time there was a lot of uncertainty regarding the circumstances, but I feel it’s time to follow up on our original announcement with what has since come to hand. I hope this will answer any outstanding questions, ease some of the concern, and most importantly I hope everyone checks their installations to make sure they are not vulnerable.

First and foremost, I can confirm that the code was malicious and the release was modified on the server by a 3rd party. Therefore, it is crucial that you follow the instructions in the previous blog post to ensure your installation is not vulnerable. The release package was obviously cleaned as soon as the alarm was raised, so if you downloaded MyBB after the first blog post then you don’t need to worry. We aren’t sure exactly when the release packages were tampered with, however if you downloaded your package shortly after the release then you may not have been effected either.

There was unfortunately a vulnerability in the CMS which powers the MyBB home page and downloads system. Using this vulnerability a hacker was able to add a backdoor to one of the files, allowing them to execute arbitrary PHP and manipulate the release packages. The CMS was custom written a number of years ago, however we believe a 3rd party framework used by the CMS contributed to the vulnerability. The CMS shares no code with MyBB so there should be no concern that these events indicate a vulnerability in MyBB. The server is also configured to isolate the subdomains belonging to the MyBB website, so it is unlikely that any data from the community forums or other sections of the site was compromised.

In light of these events, we are looking at making several changes. At the very least we intend to publish checksums with downloads to help identify any future releases which may have been contaminated, we are also looking into automating the verification process using a remote server. Using a CDN to distribute our packages is another option being considered.

MyBB 1.6.5 should be released in the next few weeks but until then please be sure to follow the instructions in the first blog post to secure your board.

15 thoughts on “Some closure on the 1.6.4 Security Vulnerability

  1. By that insomnic post i mean that static HTML is much more secure, and mediawiki when maintained is. I don’t really see the need for a full CMS for a such a static site.

  2. This is really scary. Maybe you can deliver a check sum so that effected files can at least be found faster. Except those crackers also alter the check sum on your web page.

  3. I only have one question.

    Why, oh why is there an *eval* statement in the main code in the first place?

    Other systems do use eval, true, but it’s done there primarily as a sanity check on the templates prior to executing them – they don’t use eval to actually perform live code execution. I’d love to hear why eval() is being used to populate a variable.

  4. @Arantor:
    1) It is how the template system is designed because it is an easy way to execute the template without passing all variables to it first.
    2) The hacker used that eval() for his malcode but the initial code modification was somewhere else (without using eval).

  5. Um… I said I didn’t know MyBB, but I’m not ignorant of how PHP works, so if you’re going to make a statement like ‘it is how it is designed’, I consider that fair game to pull apart in terms of how PHP as a language works.

    The statement in question is:
    eval(“$loginform = “”.$templates->get(“index_loginform”).””;”);

    Why are you doing an eval on that? When that’s evaluated, it just evaluates to a simple assignment to $loginform of the return value of that get method call. Which means that not only are you entertaining redundant code, you’re even doing it at something like 1/10 the speed of the straight assignment. I’d also point out that for bonus points, ” is used, which slows it down even more.

    You could replace that with
    $loginform = $templates->get(‘index_loginform’);

    And it would do the same thing. It’d be more readable, more maintainable and faster to boot.

    If this form of statement is going to be used, it should be using ‘ instead of ” to avoid the overhead of the complex expression parser (since you can’t use {} syntax on it as it’s a method call), and really that should be done throughout for performance (and it would save you escaping ” in such expressions if you chose to go down that route)

    In this particular case, if there’s already code injection, not having the eval wouldn’t prevent the value of that magic variable being used, but I’m just staring at it trying to understand why eval is being used for a purpose totally against that for which it was implemented.

    I also don’t see how it’s “an easy way” to execute a template… you’re still executing it. Either it’s ready to roll or it’s not, and if it’s not, why are you even running it in the first place? It’s still running the function twice.

    It sounds to me very much like there’s something deeply wrong if using eval is considered to be the *best* solution to that particular problem.

  6. @Arantor

    Maybe you should cut down on the fancy english and think before you speak (or rather write, in this case).

    They’re eval’ing it becuase they need the variables within the return value of the get() function call to be assigned values. The get() function returns a static string containing the template and the placeholders for content, in the form of variables written as text. Eval’ing that string allows the variables to be assigned values.

    Being clever is easy when you don’t know what you’re talking about.

Comments are closed.