croxton / Stash

Stash allows you to stash text and snippets of code for reuse throughout your templates.
GNU General Public License v3.0
198 stars 20 forks source link

Security issue with cache poisoning #149

Closed akolinski closed 7 years ago

akolinski commented 7 years ago

When viewing which variables are created when using the {exp:stash:cache} tag within Mustash you can create URIs that return a page (e.g. when using {last_segment} to select the entry) so it doesn't return a 404 but isn't a correctly formed URI e.g. correct path = domain.com/path1/entry_url_title, incorrect but working domain.com/path2/entry_url_title. For each unique URI a cache entry is created in the database.

Even when using context and name parameters, rather than @URI it appears to generate a cache entry because the URI is different.

{exp:stash:cache bundle="mybundle" context="the_context" name="{last_segment}" refresh="1440" parse_stage="both"}

This appears to create an attack vector for filling a database server with bogus cache entries.

You can fix this with a patch to line 2668 within mod.stash.php. I've also created a pull request https://github.com/croxton/Stash/pull/150.

$this->EE->TMPL->tagparams['context'] = $this->EE->TMPL->fetch_param('context', '@URI');

Happy to discuss further 👍

croxton commented 7 years ago

The cache can indeed be bloated by malformed or malicious urls, however it is not the job of a caching layer to police the routes into an application. You should ensure that non-valid urls always return a 404 response.

This can be handled in EE templates:

{!-- no results --}
{if no_results}
   {redirect="404"}
{/if}

{!-- additional segments --}
{if segment_4}
   {redirect="404"}
{/if}

But I highly recommend using Resource Router to define and validate known routes:

'blog/:url_title' => function($router, $wildcard) {
   // valid blog entry (channel 2)?
   if ($wildcard->isValidUrlTitle(array('channel_id' => 2))) {

      // route to template
      $router->setTemplate('blog/post');
   }
},

// any other rules here

// send any other non-matching routes to 404
'.*' => function($router) {
    $router->set404();
},

If you want to specify a specific name and context for a full-page cache you can use {exp:stash:set}:

{exp:stash:set
   name="my_name"
   context="my_context"
   scope="site"
   save="yes"
   parse="yes"
   parse_depth="4"
   refresh="0"
   replace="no"
   output="yes"
}

  // stuff to cache

{/exp:stash:set}

Please see the resource router and caching sections of this presentation for further guidance: https://speakerdeck.com/croxton/stash-the-right-way