wpsharks / comet-cache

An advanced WordPress® caching plugin inspired by simplicity.
https://cometcache.com
GNU General Public License v3.0
77 stars 18 forks source link

Feature: Allowlist-based GET query parameter caching #850

Open lkraav opened 7 years ago

lkraav commented 7 years ago

(From https://github.com/websharks/comet-cache-pro/pull/293#issuecomment-261216071)

639 has landed implementing a blocklist for optimized GET query parameter caching.

One of my scenarios would actually benefit from allowlisting instead.

COMET_CACHE_IGNORE_GET_REQUEST_VARS === *

COMET_CACHE_ALLOW_GET_REQUEST_VARS === [ 'date', 'location' ] etc.

In this scenario, the default is to route cached requests as if the request never had query parameters.

Only if ALLOW list exists, those specific parameters would create new cache entries.

This would lower DoS surface quite a bit, helping eliminate the possibility that an attacker could hit URLs with query strings just to avoid your cache engine and eat up more processing time.

To be clear, this stops untargeted random stuff. If someone targets a specific site, they will spend time to find out how to burn cycles within allowed GET parameters, too. But since the followup implementation costs here for having allowlisting have become negligible, I think overall a clear net win to be had.

lkraav commented 7 years ago

PS I'm looking at our high-traffic content marketing blog analytics at http://conversionxl.com and filtering for query parameters. The incoming marketing toolset used (or just tested) over time can be massively wide, it's not feasible to maintain a blacklist of even legit set of parameters. This confirms that a global blacklist with a controlled allow list is the way to go.

raamdev commented 7 years ago

@lkraav Thank you for opening this GitHub issue. :-)

@jaswsinc Could I get an estimate on this?

jaswrks commented 7 years ago

Estimate: Two days


Note to self: This feature, when it's made available, could be prone to misuse. So we need to be careful about how we introduce this and provide some warnings. Silently ignoring all GET variables in the cache by default can easily lead to cache files that are served for future requests with query strings interpreted by plugins, themes, or by WP core itself, and if the query string is effectively ignored by Comet Cache, a cache file could be served without those variables taken into consideration as they should be. Site owners need to know exactly what they're doing to use this.

lkraav commented 7 years ago

It could be an incremental release process. First step for implementing the necessary logic update could provide only a developer plugin API to hook into for enabling. This would allow at least one cycle of proper vetting by only knowledgeable people.