Magento 1 Netreviews/Avisverifies Security Flaw

After browsing again Magevulndb repository I encountered issue and PR open due original reporter have went 404. So I decided to give some help for these guys! Anyways, everybody wins when issues are reported.

Discovering the Flaw ​

Let’s dig trough this module and try find some issues. First I will run this trough RIPS.

One, which I was actually expecting to see is following:

This is what we call Object Injection and it has CWE-502, if you’re not familiar with this kind of injections, the principle itself is explained here.

Identifying attacks ​

Here, I will be borrowing information from this ticket combined with reading trough the source. I have not tested this trough, so this is fully theoretical.

First, the attack URL is /index.php/AvisVerifies/dialog/index/ as POST. This is totally intended URL for this module and blocking this URL will not help.

The injection itself comes from parameter called message. In this parameter you can input correctly serialized PHP code which will allow you to execute code inside Magento.

Due parameter need to be correctly, we cannot check for specific way serialized code and let it pass via HTTP server. Only option is to apply quick fix, which is not recommended or update to at least 2.16.3.

Security Flaws in latest version ​

These security flaws apply for at least version 2.16.5. I will be updating this post with correct fixed version, when these security issues has been fixed.

XSS Injection ​

So, well. Shame shame. After running RIPS on the updated version I was able to find second problem.

That is probably most common type of security related bugs. XSS Injection, aka CWE-79.

How we can use this? Well, we can give link with x value, where it will be injected into the user HTML. This allows us to inject malicious scripts and whatever we want via store -parameter.

This issue has been reported and it is probably going to be fixed in future versions.

Possible Data Leak ​

Unwritten rule in Magento (and in all PHP Applications) is never to var_dump anything in production. Well, somebody has not heard about this. Why you should not do it? Well, there is possibility you expose confidential data to your site visitor.

As far I understand, this is on admin side, so it shouldn’t be big thing. However, it’s something to note about.

Also, when I went trough the code, I noticed many places, where there was try-catch -statement catch part empty. If you’re leaving it empty, please comment, why you did it. It every other case, you should log exception to help in debug process and to identify problem causing exception itself.

The Solution ​

Solution to this problem is to upgrade latest version.

Latest version is currently 2.16.5 and CWE-502 related issues are fixed from it. However, possible data leak scenario on admin side and CWE-79 on client side still exists.

Issue has been reported to author of this module on 21.7.2019.