Mod_Security Rule Issue with Comet Cache Pro Updater

[seozones]

Hi there. I put in a request regarding this issue awhile back and it seems the issue is still unresolved. When I use the Pro Updater, it redirects to a 404 error page (page cannot be found). My techs have to disable a Mod_Security rule in order for me to update the plugin automatically using the Pro Updater which is a pain because I have many sites across many servers so it gets old asking them to disable the security rule for each of my sites. My tech has told me this (back on Nov 18):

It was mod_security rule #340163, here's the full line from our logs:

[Tue Nov 18 18:30:18 2014] [error] ModSecurity: Access denied with code 403 (phase 2). Match of "rx ://%{SERVER_NAME}/" against "ARGS:quick_cache__update_zip" required. [file "/usr/local/apache/conf/mod_sec/mod_sec.asl.conf"] [line "547"] [id "340163"] [rev "289"] [msg "Atomicorp.com UNSUPPORTED DELAYED Rules: Remote File Injection attempt in ARGS (MM)"] [data "http://wsharks.com/1vlhatG?file=quick-cache-pro-v141110.zip [1]";] [severity "CRITICAL"] [hostname "www.goldleafcare.com [2]";] [uri "/wp-admin/update.php"] [unique_id "VGvWisYL08cAAAOvAOkAAAAN"]

However, I was told that the issue would be resolved in a near future release. Any update on that fix?

[raamdev]

@seozones Thank you for the report.

ModSecurity has been known to cause issues like this with many different plugins; ModSecurity is designed to be overly cautious and often ends up blocking valid requests. We generally recommend requesting that your web host whitelist the specific rule causing problems (in this case #340163) so that the Pro Updater can run properly or, if you'd rather not have any rules disabled, to manually download and update Quick Cache Pro whenever an update is available.

That said, I'm going to leave this GitHub issue open so that we can do some more research and see if it's possible to get around this specific ModSecurity rule by changing how the Pro Updater works.

[seozones]

That would be great if you can find a workaround. Thanks raamdev!

[jaswrks]

Remote File Injection attempt in ARGS (MM)"] [data "http://wsharks.com/1vlhatG?file=quick-cache-pro-v141110.zip [1]";]

@raamdev This URL is actually base64 encoded by Zen/Quick Cache. I see in this report that it is raw. Do you know if ModSecurity is capable of decoding the value? I'm guessing that is what happened here.

[jaswrks]

Referencing this line in a private repo.

[raamdev]

Do you know if ModSecurity is capable of decoding the value?

Yes, it is. The Base64 decoder in ModSecurity is based off the RFC-4648 implementation of Base64. It most likely decodes Base64-encoded strings because obfuscating malicious code by Base64-encoding it is a very common thing, especially in PHP scripts. My guess is ModSecurity decodes the string for logging and/or comparison against known attack vectors.

[jaswrks]

What Needs to Happen Here?

• We need to use proprietary encryption on this string, or store it in a transient and reference it by ID.

[jaswrks]

Adding a list of items (↑ above) that need to occur.

[raamdev]

Noting that we came up for a workaround for this issue (in the private team repo) that utilizes WP Transients. This issue is now ready for work:

---

In proUpdate() we could modify the existing behavior to store the zip file URL in a WordPress Transient:

// [...]
GLOBAL_NS.'_update_pro_version' => $product_api_response->pro_version,
// GLOBAL_NS.'_update_pro_zip'     => base64_encode($product_api_response->pro_zip),

$is_multisite  = is_multisite();
$get_transient = $is_multisite ? 'get_site_transient' : 'get_transient';
$set_transient = $is_multisite ? 'set_site_transient' : 'set_transient';

// Store the Pro Zip URL in Transient with a 5 minute expiration
if (false === ($get_transient(GLOBAL_NS.'_update_pro_zip_'.$product_api_response->pro_version))) {
                $set_transient(GLOBAL_NS.'_update_pro_zip_'.$product_api_response->pro_version,  $product_api_response->pro_zip, 60*5);
}

Then on the other side (in preSiteTransientUpdatePlugins()), we could do the following:

if (empty($_r[GLOBAL_NS.'_update_pro_version'])) {
        return $transient; // Nothing to do here.
    }

$is_multisite  = is_multisite();
$get_transient = $is_multisite ? 'get_site_transient' : 'get_transient';
$set_transient = $is_multisite ? 'set_site_transient' : 'set_transient';
$delete_transient = $is_multisite ? 'delete_site_transient' : 'delete_transient';

if (false === ($update_pro_zip = $get_transient(GLOBAL_NS.'_update_pro_zip_'.$_r[GLOBAL_NS.'_update_pro_version']))) {
        return $transient; // Nothing to do here.
    }

// Clean up; transient value has been retrieved above if we got this far, so let's delete it.
$delete_transient(GLOBAL_NS.'_update_pro_zip_'.$product_api_response->pro_version);

$update_pro_version = (string) $_r[GLOBAL_NS.'_update_pro_version'];
// $update_pro_zip     = base64_decode((string) $_r[GLOBAL_NS.'_update_pro_zip'], true);

[jaswrks]

Noting that we can avoid the complication of _site_ or not in those transient calls, because it's safe to use get_site_transient() and get_site_option() in Comet Cache; i.e., CC is a network-only plugin on a Multisite installation, so it simplifies that for us automatically.

[raamdev]

because it's safe to use get_site_transient() and get_site_option() in Comet Cache; i.e., CC is a network-only plugin on a Multisite installation, so it simplifies that for us automatically.

I'm sorry, that's not clear to me. Are you saying that we can use get_site_transient() and drop the usage of get_transient() even if we're not on a multisite network? (And if that's not correct, then how can we avoid the complication? Don't we need to consider non-Multisite and Multisite here?)

[raamdev]

@jaswsinc Also, while testing the above code, I'm getting "The plugin is at the latest version." whenever I try to run the Pro Plugin Updater (upgrading from v160417 → v160521). That message is coming from wp-admin/includes/class-wp-upgrader.php. I'm still trying to figure out what's going on, but I know you've spent more time with this area of the codebase so I thought I'd see if you have any ideas. :-)

[jaswrks]

Are you saying that we can use get_site_transient() and drop the usage of get_transient() even if we're not on a multisite network?

That is correct, yes. Those functions work on both types of installs with graceful/logical fallbacks. So you can just use get_site_option() and get_site_transient() explicitly without any further consideration in the CC plugin.

[raamdev]

Next Release Changelog:

• Enhancement: The Pro Plugin Updater has been improved to allow for better compatibility with hosting platforms that use Apache's ModSecurity. In some cases, site owners were seeing a 404 error when attempting to update the Pro version using the Pro Plugin updater because certain ModSecurity rules were blocking the Pro Updater requests. The Pro Plugin Updater now uses WP Transients to store the necessary metadata, which works around the issue with ModSecurity. Props to @seozones for reporting and @jaswsinc for help fixing this. Issue #416.

[raamdev]

Comet Cache v160706 has been released and includes changes from this GitHub Issue. See the v160706 announcement for further details.

---

This issue will now be locked to further updates. If you have something to add related to this GitHub Issue, please open a new GitHub Issue and reference this one (#416).