flush=1 waiting to be exploited?

[Ralph Slooten]

Hi guys,

Please excuse my ignorance if this has been discussed before, however I really see no point is SilverStripe having the ?flush=1 functionality available to anyone who is not logged in. 

I actually see this as a potential accident waiting to happen, as its huge CPU / disk activity per load (about 20x as long as a regular page load on our production server) could quite easy be abused to cripple a server simply by hammering a SS website with ?flush=1

Is there any particular reason this functionality is in fact allowed without any login requirement (like flush=all)?

Kind regards,

Ralph

[Colin Burns]

You do need to be logged in to run it. It redirects you to the login page if you aren't already logged in. Well it does in 2.4.7 anyway.

Cheers,
Colin 

 

Colin Burns
Phone: +61 7 3102 4334
Fax: +61 7 3041 6080

Malaysia Mobile: +60 12 7700 564

Twitter: @ccburns
Email: ccb...@gmail.com
Web: http://www.ourtravellifestyle.com
Web: http://www.vagabondfamily.org
Web: http://www.nunomad.com
Web: http://www.onurwaytravel.com
Location: Penang, Malaysia
GMT: +8hours

--
You received this message because you are subscribed to the Google Groups "SilverStripe Core Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to silverstripe-d...@googlegroups.com.
To post to this group, send email to silverst...@googlegroups.com.
Visit this group at http://groups.google.com/group/silverstripe-dev?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Ralph Slooten]

On Thu, Feb 28, 2013 at 7:05 PM, Colin Burns <ccb...@gmail.com> wrote:

You do need to be logged in to run it. It redirects you to the login page if you aren't already logged in. Well it does in 2.4.7 anyway.

You do not need to be logged in versions 3.0.* - only for ?flush=all

[Daniel Hensby]

To be honest, this has always been something to concern me too.

As silverstripe becomes more popular, this needs to be stopped.

Dan

--

[Ingo Schommer]

I've created a ticket for it: http://open.silverstripe.org/ticket/8290

Wouldn't go so far to say its a security issue, but a grave oversight

which leaves SilverStripe installations unnecessarily exposed.

By the way, anybody keen to write a dev/clearcache task?

Ideally with optional cache names to clear, and an overview

of cache age and expiry rules on different caches.

I don't really like this whole magic GET parameter business.

We'd need to extend SS_Cache to keep track of available caches for this.

There's also a related annoyance around partial caches not being cleared with ?flush=1,

see http://open.silverstripe.org/ticket/5807

[Nicolaas Thiemen Francken - Sunny Side Up]

On 28 February 2013 23:16, Ingo Schommer <in...@silverstripe.com> wrote:

I've created a ticket for it: http://open.silverstripe.org/ticket/8290

Wouldn't go so far to say its a security issue, but a grave oversight

which leaves SilverStripe installations unnecessarily exposed.

 

By the way, anybody keen to write a dev/clearcache task?

Ideally with optional cache names to clear, and an overview

of cache age and expiry rules on different caches.

I don't really like this whole magic GET parameter business.

We'd need to extend SS_Cache to keep track of available caches for this.

There's also a related annoyance around partial caches not being cleared with ?flush=1,

see http://open.silverstripe.org/ticket/5807

I did a quick search for flush and found the following references... 

ine 295 of Core.php (using 3.1.0-beta2):

SS_TemplateLoader::instance()->pushManifest(new SS_TemplateManifest(

BASE_PATH, project(), false, isset($_GET['flush'])

));

line 1034 of Requirements (using 3.1.0-beta2):

if(file_exists($combinedFilePath) && !isset($_GET['flush'])) {

(using 3.1.0-beta2)

line 659 of SSViewer: 

if (isset($_GET['flush']) && $_GET['flush'] == 'all') {

if(Director::isDev() || Director::is_cli() || Permission::check('ADMIN')) {

self::flush_template_cache();

} else {

if(!Security::ignore_disallowed_actions()) {

return Security::permissionFailure(null, 'Please log in as an administrator to flush ' .

'the template cache.');

}

}

}

[Ralph Slooten]

On Thu, Feb 28, 2013 at 11:16 PM, Ingo Schommer <in...@silverstripe.com> wrote:

I've created a ticket for it: http://open.silverstripe.org/ticket/8290

Thanks Ingo for looking into this. Yes I realise that is one has a borked cache that things can become very difficult without console cli access to the site,and I assume your proposal of the Database::is_ready() is probably a good way to circumvent the potential deadlock, though I don't know the core infrastructure well enough to pass comment on that.

I appreciate your quick response!

Kind regards,

Ralph

[Anselm Christophersen]

I'm using this on some sites:
https://gist.github.com/anselmdk/5064660

Actually, I think, I took it from Mark Guinn's repo, so he should be attributed for it.

[Damian Mooyman]

I've posted a sample flush-checking function on the ticket at https://github.com/silverstripe/sapphire/issues/1692#issuecomment-17649637

How does the below look?

// In Director.php

    /**
     * Check requested flush level, given permission is available, or the 
     * database is not ready.
     * 
     * @param string $value An optional flush value to require. E.g. 'all'
     * @return boolean A flag indicating that the requsted flush level is given and authorised
     */
    public static function is_flush($value = null) {

        // Rule out unflushed cases
        if(empty($_GET['flush'])) return false;
        if($value && ($_GET['flush'] != $value)) return false;

        // Allow flush in dev or CLI
        if(self::isDev() || self::is_cli()) return true;

        // If a database error would otherwise prevent authentication, permit flush
        if(!DB::isActive() || !DB::getConn()->hasTable('Member')) return true;

        // Safely check permission, erring on the side of safety
        try {
            $permission = @Permission::check('ADMIN');
            if(is_bool($permission)) return $permission;
        } catch(Exception $ex) {
            exceptionHandler($ex);
        }

        return true;
    }

[Daniel Hensby]

Your first line to rule out unflushed cases doesn't actually do what it says. Currently in SS there is no obligation for the flush param to have a value. Ie example.com?flush works. Removing that behavior will take some getting used to.

So, I'd suggest the first check is an isset, not an empty.

Also, my understanding is there are two non-falsey values flush can take, 1 and all. I'm not sure of their exact intricacies, but all does everything 1 does and more. With your function, if I'm checking for flush=all when the flush value is 1, I would get false, when I should get true.

I hope that all makes sense, I'm on my phone so formatting isn't so easy!

Dan

--

[Damian Mooyman]

Sorry, I was under the impression flash had to be given a value, and that ?flush by itself meant no flush.

There's also issues with bootstrapping in the function above, since flush can used when the session isn't ready. I'm going to politely retreat until I have a better solution. :)

If you called is_flush('all') that would mean you REQUIRE the 'all' key. In this case you should be calling it without parameters. The all/1 is a stupid vague hook anyway.

https://groups.google.com/forum/?fromgroups=#!topic/silverstripe-dev/PdPR_iFSztE Required reading

Damian