Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Protection from object injection in cart
  • Loading branch information
opencarthelp committed Jun 5, 2014
1 parent 56981bf commit c2aafc8
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions upload/system/library/cart.php
Expand Up @@ -265,7 +265,7 @@ public function getProducts() {
}

public function add($product_id, $qty = 1, $option = array()) {
if (!$option) {
if (!$option || !is_array($option)) {
$key = (int)$product_id;
} else {
$key = (int)$product_id . ':' . base64_encode(serialize($option));
Expand All @@ -283,7 +283,7 @@ public function add($product_id, $qty = 1, $option = array()) {
}

public function update($key, $qty) {
if ((int)$qty && ((int)$qty > 0)) {
if ((int)$qty && ((int)$qty > 0) && isset($this->session->data['cart'][$key])) {
$this->session->data['cart'][$key] = (int)$qty;
} else {
$this->remove($key);
Expand Down

6 comments on commit c2aafc8

@fgeek
Copy link

@fgeek fgeek commented on c2aafc8 Jul 25, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create new release to include this patch, thank you. This is serious security vulnerability.

http://osvdb.org/109043

@danielkerr
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its not a serious security vulnerability!

@fgeek
Copy link

@fgeek fgeek commented on c2aafc8 Aug 5, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Object injection is a serious security vulnerability, but OSVDB description provides more information:

OpenCart contains a flaw in the Cart::getProducts() method in the cart.php that is triggered as input is not sanitized when passed via the 'quantity' parameter when handling update requests. This may allow a remote attacker to conduct a server side request forgery (SSRF) attack.

@tyronx
Copy link

@tyronx tyronx commented on c2aafc8 Sep 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've been getting responses from customers about strange emails containing viruses sent from the webshop email addresses. According to the description of SSRF attacks, this vulnerability would allow an attack to do exactly that.

I applied the patch and hope that this stops the sending of malware with a seemingly valid sender email. In any case I would also urge the OpenCart Team to create a new release containing this patch.

@akonstatinos
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tyronx did the patch solved the problem with the strange emails containing viruses?
As I have same problem with strange emails I investigate the possibility that this vulnerability causes the problem.

@IP-CAM
Copy link

@IP-CAM IP-CAM commented on c2aafc8 Sep 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In all OpenCart 1.5.6.5_rc Versions, only the second mentioned FIX would be required in
the:
public function update($key, $qty) {
Section.
The other routine shown above does not exist anymore in 1.5.6.5_rc OC Source.

This is a VqMod Section, to make it work at once:

<operation error="log">
<search position="replace" index="2"><![CDATA[if ((int)$qty && ((int)$qty > 0)) {]]></search>
<add><![CDATA[
if ((int)$qty && ((int)$qty > 0) && isset($this->session->data['cart'][$key])) {
]]></add>
</operation>

Please sign in to comment.