BSS Multiwishlist XSS Injection

This time I have been moving to weird and good Magento 2 world. How is it good? I have noticed (at least on those modules I’ve gone trough) there is a lot less issues in the code. Starting from best practices to real big good old security issues. However, there is still issues, even tough not so great like plain SQL injections. Or maybe it is just due I have been auditing the only big vendor’s modules and not yet the smaller ones.

The Injection ​

There is a injection, but I would to be more exact, there is actually 3 of them. Injection belongs to category of CWE-79 which means it is XSS injection.

Just a refresh our minds, a reflected cross-site scripting (XSS) vulnerability occurs when unsanitized user input is embedded into the HTML response page of the web application. It allows an attacker to inject arbitrary HTML or JavaScript code into the response page of a tampered request. Usually, this attack is performed by crafting a malicious link that is sent to a victim. When opened, the attacker’s JavaScript payload within the link is reflected by the application and executed in the victim’s browser in the context of the web application’s domain. This enables the attacker to perform phishing attacks, to steal cookies associated with the domain, or to cause the victim’s browser to execute arbitrary actions on the victim’s behalf and without the victim’s knowledge.

Preventing and Detecting ​

To prevent cross-site scripting vulnerabilities, special characters that are interpreted by the browser to execute not intended actions need to be escaped or filtered out of user input before usage. Which characters are considered harmful and need to be sanitized depends on the context the injection happens in (e.g., attribute context, URL context, JavaScript context, …).

The detected injection occurs within a double-quoted HTML attribute. An attacker can break out of this attribute by injecting a double quote ("). This allows to terminate the current attribute and to append another attribute to the HTML element. For example, an eventhandler attribute can be appended that allows to execute arbitrary JavaScript code.

The detected cross-site scripting vulnerability occurs within the context of an attribute surrounded by double quotes. To prevent abuse, it is necessary to prohibit the potentially malicious input from breaking out of this context, and inject an event handler or start a new HTML tag. The PHP built-in function htmlentities() can be used for this matter. While escaping of single quotes is not necessary at this point, it is still recommended to do so by adding the ENT_QUOTES flag to the call to htmlentities().

Injections in the Module and fixing ​

First injection have been found at view/frontend/templates/popup.phtml on line of 33. There we can find following code.

<input type="hidden" value="<?php echo $block->getRequest()->getParam('product_id'); ?>" id="productId"

And on the line 35

<input
  type="hidden"
  value="<?php echo $block->getRequest()->getParam('itemid'); ?>"
  id="itemid"
  name="itemid"
/>

The code in both are pretty same. Both are hidden input fields. Both have value which is taken from the request. — Wait! From a request!?

Okay, most of you know what this means, but let me explain. Basically the code calling method getParam is taking given value from request URL. If I have following url https://magento.xx/index.php?product_id=somescripthere and then I share it to my victim, there is a injection done.

That was easy, but then comes the harder part, Preventing injection, which at least seems to be the harder part due I need to even write this blog post.

Like I mentioned before, to prevent abuse it is necessary to prohibit the potentially malicious input from breaking out of this context, and inject an event handler or start a new HTML tag. At simplest form we are just using built in PHP function htmlentities(). See the example bellow.

First fix for a line 33.

<input type="hidden" value="<?php echo htmlentities($block->getRequest()->getParam('product_id'), ENT_QUOTES); ?>" id="productId"

and for the line 35.

<input
  type="hidden"
  value="<?php echo htmlentities($block->getRequest()->getParam('itemid'), ENT_QUOTES); ?>"
  id="itemid"
  name="itemid"
/>

These fixes are not big to do. These do not take more than 1 minute of developer time to fix. Why does this still happen? Reason is simple, there is not enough knowledge within the developer to know about if these injections. That should be prevented by strict review process by other senior developer, which some companies do enforce.

Last injection quickly ​

Just to mention also about last injection, this is same as two earlier. There is on file view/frontend/templates/sharing.phtml at line 32 same XSS injection as before. Vulnerable code is following

<input
  class="mwishlist_id"
  name="mwishlist_id"
  id="mwishlist_id"
  type="hidden"
  value="<?php echo $block->getRequest()->getParam('mwishlist_id') ?>"
/>

and the fixing code for this injection is like

<input
  class="mwishlist_id"
  name="mwishlist_id"
  id="mwishlist_id"
  type="hidden"
  value="<?php echo htmlentities($block->getRequest()->getParam('mwishlist_id'), ENT_QUOTES) ?>"
/>

As you can see, its not a big and its in 98% of cases same htmlentities with ENT_QUOTES flag you can utilize.

Reporting issue ​

I have reported this issue to BSS on 5.12.2019. They have received it in form of ticket. However, even I would like everything went smoothly; they are hiding behind the payment wall. What does this mean, let me explain further.

This is paid module by other guy, who sent this for a audit to me. Ofc, I only do that I’m requested to do with it. Audit it for a security issues. I also report this issue always to the vendor of the module. Usually vendors are very happy about getting know their security issues and fix them quickly. However, this time they are refusing to even research it. Reason behind this action is, I have audited older version of this module and they are refusing to do anything about this unless I do the audit again on newer version of this module.

Firstly, the version I did audit was 1.1.4 and after looking the changelog, there is no mention whatsoever about fixing a security issue.

Changelog as of 5.12.2019

At this moment I’m fighting with them about paying for this module, so I can audit their mistakes in the code. This doesn’t seem really right as they should be paying me for doing this audit, if we are honest here. BSS is getting here more value to their product when I report it, let them to fix it and publish blog post with their support, but not this time as they refused to co-operate.

BssCommerence has also said they are lowering this security issue related ticket priority as it is not reported on certain customer live site. It seems pretty weird as their module page says this module is downloaded 1000+ times, meaning it should be live at least 80% of those.

Overall Thoughts ​

Overall it seems we have here one company which is not interested about their product security whatsoever. Even by looking their ToS, you can find section, where they put liability to the customer about the module issues. Having a module to do specific functionality while having a security issue is not a worth. Having your shop hacked will have huge impact on customer base.

Module Status at this moment Current status of module is still Vulnerable but fix available. I highly recommend to check your module version and update to at least 1.2.0.

Update: Today (7.12) they have finally informed me after releasing this blog post, there was issue indeed. (Come on, why need to go public and release blog to get company notice their mistakes!) They have promised me to release patch on this weekend. I’m still somehow surprised it takes long to fix it as there is fix in my blog post. Fixing takes less than 5 mins and releasing patch max 1 hour… Informed Magento marketplace about this.

Update: 1.2.0 fixes XSS issues. (11.12.2019)

You can follow updates at official website.