vulnerability on PrestaShop sites

[e-com]

Do not modify smarty.config.inc.php as recommended by PrestaShop, because it is a nonsense fix.
This patch encrypts data written to the PREFIX_smarty_cache table, preventing SQL injection attacks.
https://github.com/thirtybees/thirtybees/commit/f215fd84a59b6fee6f4eae4cae190101c926be67

 

[Smile]

We plan to send the brand new 1.4 version out in upcoming week(s) and I expect this or another fix to this issue to be included. 

We actually plan Monday but we have to see 😉

[Acer]

2 hours ago, e-com said:

Do not modify smarty.config.inc.php as recommended by PrestaShop, because it is a nonsense fix.
This patch encrypts data written to the PREFIX_smarty_cache table, preventing SQL injection attacks.
https://github.com/thirtybees/thirtybees/commit/f215fd84a59b6fee6f4eae4cae190101c926be67

 

Does this work in TB. Can we apply verbatim? 

[the.rampage.rado]

11 hours ago, led24ee said:

I can't use Filesystem cache, because I run out of inodes. I just disabled cache at all. Ans i f i look my log files then more than week almost every day there is one or two try to put something in some database.

Why don't you cron a purge operation at certain intervals with file system?

[e-com]

1 hour ago, Theo said:

Does this work in TB. Can we apply verbatim? 

_PHP_ENCRYPTION_KEY_ key and PhpEncryption library have been in Thirty Bees since version 1.0.0.
I have already implemented this modification on several stores in versions 1.3.0 and 1.4.0. And it certainly works fine.

[datakick]

4 minutes ago, e-com said:

_PHP_ENCRYPTION_KEY_ key and PhpEncryption library have been in Thirty Bees since version 1.0.0.
I have already implemented this modification on several stores in versions 1.3.0 and 1.4.0. And it certainly works fine.

Not necessarily. TB supports multiple encryption methods, so php encryption key might not be defined. Use Encryptor::getInstance() instead.

Encryption and decryption of cache entries might be performance killer, though. I believe signature will be a better alternative here. Will do some benchmark testing.

[e-com]

23 minutes ago, datakick said:

Encryption and decryption of cache entries might be performance killer, though. I believe signature will be a better alternative here. Will do some benchmark testing.

It's a fact, all benchmarks have shown me that a smarty cache for MySQL is much slower than a cache for a filesystem (and with encryption it will be even slower).
That's why I do not use smarty cache for MySQL in any store 🙂

Fix is just so that there is no security hole.

Edited July 28, 2022 by e-com

[datakick]

5 minutes ago, e-com said:

It's a fact, all benchmarks have shown me that a smarty cache for MySQL is much slower than a cache for a filesystem (and with encryption it will be even slower).
That's why I do not use smarty cache for MySQL in any store 🙂

Fix is just so that there is no security hole.

yep, depending on a benchmark test results this functionality could be dropped completely. It is likely that added overhead of db query + decryption will erase any perfomance gains this cache has to offer 

[e-com]

On 7/28/2022 at 2:09 PM, datakick said:

Not necessarily. TB supports multiple encryption methods, so php encryption key might not be defined. Use Encryptor::getInstance() instead.

Encryptor is a new class in thirty bees, not existing in prestashop, so I missed it.
It will be better this way:
https://github.com/thirtybees/thirtybees/commit/ab1ebcbb181913d9b1400d2d95e468241e2be025

And a performance test from profiler. Smarty cache to file system is definitely more efficient.

[datakick]

The proposed fix is in the bleeding edge, as a quick solution to this very serious issue.

Note #1: there is a significant performance overhead of encryption + base64 encoding / base64 decoding + decryption, which may render this cache totally useless. I did not make any performance tests yet, but plan to do so. If the test shows that the performance overhead is too big, we will drop this functionality completely. Tracked here: https://github.com/thirtybees/thirtybees/issues/1476

If you are using mysql smarty cache, please test yourself, and decide if different cache should be used instead.

Note #2: thirty bees already supports various server side caches (redis, apc, memcached), but this SSC is used only for Full Page Cache functionality. We should reuse this for smarty cache as well, which could give us quite a big performance boost. Tracked here https://github.com/thirtybees/thirtybees/issues/1477

Note #3: I would like to reiterate. This Smarty MySQL Cache vulnerability can be exploited if, and only if, your store already contains some SQL injection vulnerability. And if that is the case, you already have a huge problem. We don't know about any SQL injection vulnerability in thirty bees core or any native modules, but there may be some somewhere. But any third party module can have such vulnerability.