nbpoole

Don't forgot 'password1' ;-)

And the guidelines specifically say "Blacklisted passwords should be implemented (contact infrasec for the list)" which indicates to me that known common passwords like '12345678' and 'password' will be disallowed (although we don't have access to the list).

My opinion (and we may have to agree to disagree on this point) is that adding one character to the minimum is not going to make a significant difference in application security. I don't believe it mitigates the danger of a leak of DES-encrypted passwords. If you're concerned about a scenario where a user's shared password on another site is compromised, your application can use two-factor authentication or mandate the use of strong pass-phrases instead of traditional passwords.

One interesting/cool suggestion that I think is worth noting specifically: the use of HMAC+bcrypt instead of just bcrypt for secure password storage.

https://wiki.mozilla.org/WebAppSec/Secure_Coding_Guidelines#...

- The nonce for the hmac value is designed to be stored on the file system and not in the databases storing the password hashes. In the event of a compromise of hash values due to SQL injection, the nonce will still be an unknown value since it would not be compromised from the file system. This significantly increases the complexity of brute forcing the compromised hashes considering both bcrypt and a large unknown nonce value

- The hmac operation is simply used as a secondary defense in the event there is a design weakness with bcrypt that could leak information about the password or aid an attacker

Lets think about that for a minute. DES truncates at 8 characters, so you're right that if a database with DES-encrypted passwords leaks and is brute forced an attacker will only get the first 8 characters of a password.

But what if my password is the word "biological"? By knowing the first 8 characters, the attacker has drastically reduced the number of guesses that need to be made (assuming a priori knowledge that the password is shared between sites).

Also consider MD5(PASSWORD) and SHA1(PASSWORD). Those are both fairly common constructions for "secure password hashing" [note: they're not really secure] in web applications and both of those would yield up the entire plaintext password if an attacker used a brute-force or rainbow table attack.

If you're designing a secure web application, you can't make your goal to secure all the other websites on the Internet. Bumping the minimum number of characters to 9 wouldn't significantly impact the security of your users. If you're really worried about a situation where a user's password is disclosed, you should consider offering two-factor authentication options for your users.

Except if you find an XSS vulnerability in a site, you already have a serious security vulnerability that can be abused in many different ways to compromise a user. pushState simply makes one of those ways (phishing) more effective.

Personally, I'd rather take actions that don't require user interaction, like stealing the user's session cookie silently in the background or making other requests on their behalf to the website. That can be done via an invisible iframe on an otherwise legitimate website (which, I suppose, you could also consider to be "dangerous").

You might want to read http://www.imperialviolet.org/2011/09/23/chromeandbeast.html, which sums up Chrome's reaction to the attack.

You copied it from http://www.educatedguesswork.org/2011/09/security_impact_of_...

Finding SQL injection in third party applications != "Facebook servers compromised." Flagged.

Your point was already addressed: http://news.ycombinator.com/item?id=2960504.

Google also has a security bug bounty program on its web applications: http://www.google.com/about/corporate/company/security.html and http://googleonlinesecurity.blogspot.com/2010/11/rewarding-w....

Got a demo set up somewhere?

OK, I'm having a lot of fun reading this.

One little nitpick:

> What systems programming functionality does Javascript lack?

Here's a starting point: a secure random number generator.

Chrome has actually implemented one (http://blog.chromium.org/2011/06/new-chromium-security-featu...). It's a part of WebKit (http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2011-Feb...).

Firefox has an unimplemented (as of Firefox 6) secure RNG API (https://developer.mozilla.org/en/JavaScript_crypto, https://bugzilla.mozilla.org/show_bug.cgi?id=440046).

I managed to dig up a recent comment thread by tptacek that explains why JS crypto is a bad idea:

http://news.ycombinator.com/item?id=2855556

The developer's ;-)

SQLite doesn't have strict types (see http://www.sqlite.org/datatype3.html). MySQL does. It's not SQLite's fault or ActiveRecord's fault: it's one of the many issues you have to take into account when developing software that runs on two very different database systems.

Edit: ActiveRecord specifically notes this behavior in the documentation:

http://api.rubyonrails.org/classes/ActiveRecord/ConnectionAd...

    Please be aware of different RDBMS implementations behavior with :decimal columns:

    The SQL standard says the default scale should be 0, :scale <= :precision, and makes no comments about the requirements of :precision.

    MySQL: :precision [1..63], :scale [0..30]. Default is (10,0).

    PostgreSQL: :precision [1..infinity], :scale [0..infinity]. No default.

    SQLite2: Any :precision and :scale may be used. Internal storage as strings. No default.

    SQLite3: No restrictions on :precision and :scale, but the maximum supported :precision is 16. No default.

    Oracle: :precision [1..38], :scale [-84..127]. Default is (38,0).

    DB2: :precision [1..63], :scale [0..62]. Default unknown.

    Firebird: :precision [1..18], :scale [0..18]. Default (9,0). Internal types NUMERIC and DECIMAL have different storage rules, decimal being better.

    FrontBase?: :precision [1..38], :scale [0..38]. Default (38,0). WARNING Max :precision/:scale for NUMERIC is 19, and DECIMAL is 38.

    SqlServer?: :precision [1..38], :scale [0..38]. Default (38,0).

    Sybase: :precision [1..38], :scale [0..38]. Default (38,0).

    OpenBase?: Documentation unclear. Claims storage in double.

100% correct!

The only thing I'd point out is that IE's system will disable all scripts on the page if it detects an attack. For Facebook, that means the clickjacking script will be stopped but the page will also be almost toally inoperable (due to the huge amount of JS that the site requires to function).

> In the last couple of months, Jay & Mike have been responsible for the development of Jigoshop, an exciting new commerce plugin for WordPress, as part of their previous employment gig at Jigowatt. We have decided to continue the development work on a forked version (that’s the Wikipedia link if you’re unsure what forking means) of Jigoshop, which will now be known as WooCommerce.

So yes and yes. The comments actually clarify the situation a little bit beyond that (apparently this fork was their last resort):

http://www.woothemes.com/2011/08/the-good-stuff-new-team-mem...

There were two earthquakes in the past day: one in Colorado, one in Virginia.

Or bcrypt, which is actually implemented in PHP as part of the crypt function ;-)

I just tried it out myself: I can't get DELETE to work in IE9 forms.

My code (I've tried it with both upper-case and lower-case DELETE):

    <form action="http://example.com" method="DELETE">
      <input type="submit" />
    </form>

> This works great in all modern browsers, except IE9. We discovered that not only does IE9 send a real DELETE request, it also follows the redirect with another DELETE. If that redirect points to another resource, you can get a dangerous cascading effect.

redirect_to issues a 302 redirect. According to http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html, this is what a 302 redirect means:

The requested resource resides temporarily under a different URI. Since the redirection might be altered on occasion, the client SHOULD continue to use the Request-URI for future requests. This response is only cacheable if indicated by a Cache-Control or Expires header field.

The temporary URI SHOULD be given by the Location field in the response. Unless the request method was HEAD, the entity of the response SHOULD contain a short hypertext note with a hyperlink to the new URI(s).

If the 302 status code is received in response to a request other than GET or HEAD, the user agent MUST NOT automatically redirect the request unless it can be confirmed by the user, since this might change the conditions under which the request was issued.

Note: RFC 1945 and RFC 2068 specify that the client is not allowed to change the method on the redirected request. However, most existing user agent implementations treat 302 as if it were a 303 response, performing a GET on the Location field-value regardless of the original request method. The status codes 303 and 307 have been added for servers that wish to make unambiguously clear which kind of reaction is expected of the client.

There was another discussion about the use of 302 versus 303 redirects on HN about a month ago: http://news.ycombinator.com/item?id=2791663

The original HN submission for the post it's responding to: http://news.ycombinator.com/item?id=2898087

I've also taken the time to comment on the blog post itself. Reproducing my comment here:

---------------

Thanks for the response! I just have a couple points to make:

---

"Seemingly reacting to this response, Poole’s blog post appeared the next day instead of the aforementioned 26th"

Yes. It was a reaction to the bug being marked public, which meant anyone could view the full details of the issue.

---

"Providing two good reasons for this is not difficult and I am sure there are others:

1. I don’t want to see anything to do with HTML if I’m not doing web-based programming with PHP (such as CLI).

2. I want my error messages to provide me with precise information. I do not want an error message to arbitrarily encode problematic code. If the user provided <s>test</s> then I want to see <s>test</s>. I do not want to see &lt;s&gt;test&lt;/s&gt; since that is not what was submitted and, possibly, not what the problem is."

As I pointed out on Hacker News, the CLI interpreter does not HTML encode fatal errors, warnings, or notices. The CGI interpreter DOES encode fatal errors and warnings, but not notices. As such, your points aren't really applicable to the situation.

---

"No, Mr. Poole. A decent sysadmin has better things to worry about than PHP configurations. Regardless, sysadmins should be perfectly aware of the stupidity of letting display_errors remain enabled on a production environment; the same should be said for any developer worth their salt. There simply isn’t a valid excuse and this is a perfect example of why configurations differ between development and production environments."

1. You're using a logical fallacy to make your point: http://en.wikipedia.org/w/index.php?title=No_true_Scotsman&#...

2. Many shared hosts do enable display_errors by default.

3. I'm not arguing that this is a feasible attack to pull off against most applications: it most certainly is not. I'm arguing that it shouldn't be possible to pull it off at all, even in the small percentage of applications that might be affected. There is no good reason PHP should be displaying this information unsanitized.

---

"How is this a proof of concept of an undefined index or notice?"

You seem to have missed the line

    $message = $messages[$_GET['message']];

The string 1<script>alert(1)</script> passes the initial if statement but will trigger an undefined index notice on that line of code, with the name of the index under user control.

---

"Recommending programmers turn off their error display without actually addressing the bad code is not good advice at all and something I would hardly expect from someone who works in web application security."

First you tell me my recommendation is common sense, then you tell me it's bad advice. Make up your mind ;-)

Anyway, as I said on Hacker News, my original report only focused on the "Undefined index" message, which is a fairly benign issue (albeit a sign of sloppy coding). The other examples were discovered later and were only mentioned to demonstrate that PHP notices in general had an issue.

I completely disagree with your assessment of my recommendation though. Telling people to disable display_errors prevents the issue at hand, the cross-site scripting, from ever occurring. In contrast, telling people to go find and fix any PHP notices in their code is a reactive solution: it only works until the next piece of sloppy code gets checked in, at which point the site is vulnerable again. And of course, as you've said repeatedly, display_errors should not be enabled in production in any case.

It does ;-)

http://news.ycombinator.com/item?id=2898201

Thanks, I just responded on your blog.

The former. From the GitHub description:

> WordPress, Git-ified. Synced via SVN every 30 minutes, including branches and tags!

Haha, whoops! Sorry about that, back to normal. Anyone who hit that URL in the past 10 minutes would have seen unsanitized HTML, because I was testing the impact of disabling the html_errors INI option (something one of my blog's comments asked about). I've updated the blog post to discuss html_errors as well. ;-)

> So with this I can XSS myself while in development mode on my development system? That's handy.

Or someone can send you a URL that points to your dev server and contains a payload. Or someone can send a URL to the developer across the street who uses shared hosting where display_errors is enabled by default.

I'm not arguing that this is a feasible attack to pull off against most applications: it most certainly is not. I'm arguing that it shouldn't be possible to pull it off at all, even in the small percentage of applications that might be affected. There is no good reason PHP should be displaying this information unsanitized.

---

You're 100% correct about the example you're pointing out. If you're allowing arbitrary functions called from the user, you're in much bigger trouble. ;-)

I originally noticed and reported the issue with the first example, which triggers a simple, common undefined index notice:

    <?php
    error_reporting(E_ALL);
    $array = array();
    echo $array[$_GET['index']];

I only included the others for the sake of completeness. I've updated the code sample to note that.

> So could PHP perhaps HTML encode error messages? Would this break any error message use cases? For example are there times when error messages appear when they wouldn't be interpreted as HTML encoded? CLI? Browsers loading different text mime types?

This is something PHP already gets right with warnings, fatal errors, etc. The issue is only with notices as far as I've seen (I haven't delved into the PHP source to see what the difference is).

A demonstration file:

    [cmd]$ cat 147b9119e818c92f7f74bad71cc12255.php
    <?php
    echo file_get_contents('<s>test</s>');

Compare:

    [cmd]$ php -f 147b9119e818c92f7f74bad71cc12255.php

    Warning: file_get_contents(<s>test</s>): failed to open stream: No such file or directory in /home/nbsandbox/sandboxing.me/poc/147b9119e818c92f7f74bad71cc12255.php on line 2

to

http://sandboxing.me/poc/147b9119e818c92f7f74bad71cc12255.ph...

    <br />
    <b>Warning</b>:  file_get_contents(&lt;s&gt;test&lt;/s&gt;) [<a href='function.file-get-contents'>function.file-get-contents</a>]: failed to open stream: No such file or directory in <b>/home/nbsandbox/sandboxing.me/poc/147b9119e818c92f7f74bad71cc12255.php</b> on line <b>2</b><br />

See also: http://news.ycombinator.com/item?id=2844321

That blog post seems to contradict what Tavis Ormandy claimed on Twitter a few days ago, when the patch was released:

> Adobe patched around 400 unique vulnerabilities I had sent them in APSB11-21 as part of an ongoing security audit. Not a typo.

https://twitter.com/#!/taviso/status/101046246277521409

> Apparently that number was embarrassingly high, and they're trying to bury the results, so I'll publish my own advisory later today.

https://twitter.com/#!/taviso/status/101046396790128640

Whereas the blog post cites 400 unique crashes, 106 security bugs, and 80 code changes (the same numbers that Adobe used: http://blogs.adobe.com/asset/2011/08/how-did-you-get-to-that...).

---

Regardless of the exact numbers though, this is a supremely awesome feat of security engineering. It's very impressive.

Agreed. The older a PHP application is, the more likely it is to have incredibly serious security vulnerabilities. But I've looked at and broken plenty of vulnerable applications that were written within the past couple years.

daeken is arguing, based on quite a bit of experience, that PHP applications (even those written with frameworks) tend to have more security vulnerabilities than Rails apps. I happen to agree with him.

One very illustrative example: arbitrary code execution. I've lost count of the number of arbitrary code execution vulnerabilities I've found in PHP applications. In contrast, I recall very vividly the last arbitrary code execution vulnerability I found in a Python application: I uploaded a PHP script to the server, which was also running mod_php. ;-)

The problem here is simple: most web servers that run PHP are configured with a rule that says "If a file ends in .php, execute it as PHP." This is useful for new users: it allows them to run and execute separate PHP scripts very easily. But it's also a potential security vulnerability if an attacker can upload a PHP file to your server.

If you're using a framework, you typically have a fixed number of scripts that should be executable and you can configure your web server appropriately. However, that requires a VPS or dedicated server.

Edit: If you're interested in an actual language-level difference between PHP and Ruby/Python that affects security, PHP scripts accepted null bytes as part of filesystem paths until recently (PHP 5.3.4, which added protection against it, was released at the end of 2010).