Exploits... Fix code

For articles specific to version 6.x
Post Reply
Martin
Site Admin
Site Admin
Posts: 1854
Joined: Wed Jun 17, 2009 6:30 pm
Location: South Yorkshire UK
Contact:

Exploits... Fix code

Post by Martin »

This a place holder for me for now but these exploits are real so you might want to change a few things now Tony/Jon.

Address leak/re-assignment

Problem Summary
Address Book Information Leakage - view any users addresses and change ownership

Steps to Reproduce
Log in. View Address book. Edit address - then changing the ID in the url allows for traversal of other entries.
Confirmed!

Fix:

Open: /includes/classes/class.account.php


Find:

Code: Select all

		private function SaveEditedShippingAddress()
		{
			if (isset($_POST['shipid'])) {
After, Add:

Code: Select all

				//MOD Test ownership
				$this->DLCheckAddressOwnership($_POST['shipid']);
				//MOD END
Find:

Code: Select all

		private function EditShippingAddress($MsgDesc = "", $MsgStatus = "")
		{
			if (isset($_GET['address_id'])) {
After, Add:

Code: Select all

				//MOD Test ownership
				$this->DLCheckAddressOwnership($_GET['address_id']);
				//MOD END
Find: (at end of file)

Code: Select all

	}
Before, Add:

Code: Select all

		/*
		 * Check ownership of an address record and fail if someone is attempting
		 * to spoof the system and re-assign to their own account.
		 */
		private function DLCheckAddressOwnership($shipid) {

			$dl_ship_id = (int)$shipid;

			$query = sprintf("SELECT COUNT(shipid) AS VALUE FROM [|PREFIX|]shipping_addresses WHERE shipcustomerid='%d' AND shipid='%d'", $GLOBALS['ISC_CLASS_CUSTOMER']->GetCustomerId(), $dl_ship_id); 
			$res = $GLOBALS['ISC_CLASS_DB']->Query($query);
			$count = $GLOBALS['ISC_CLASS_DB']->FetchOne($res);

			if ($count != 1) {

				// Bad details or they don't own the shipping address 
				ob_end_clean(); 
				header(sprintf("location:%s/account.php", $GLOBALS['ShopPath'])); die(); 
			}
			return true;
		} 
Martin
Site Admin
Site Admin
Posts: 1854
Joined: Wed Jun 17, 2009 6:30 pm
Location: South Yorkshire UK
Contact:

Re: Exploits... Fix code (DO NOT POST IN WILD!)

Post by Martin »

Oh it gets better...

It's apparently possible to use tools to adjust the quantity value in a basket and create a scenario where someone could apply a negative quantity value and thus essentially create a credit... Not sure how that would work with payment gateways or indeed if there's a sanity check... Not good either way.

class.cart.php
lines 272 - 278 just ensures it's an integer, no checking for +/-

Find:

Code: Select all

		$qty = 1;
		if(isset($_REQUEST['qty'])) {
			if(is_array($_REQUEST['qty'])) {
				$qty = (int)array_pop($_REQUEST['qty']);
			}
			else if($_REQUEST['qty'] > 0) {
				$qty = (int)$_REQUEST['qty'];
			}
After, Add:

Code: Select all

			//MOD Stop negative quantity values sneaking through.
			$qty = $qty >= 0 ? $qty : 1;


Find:

Code: Select all

					// if the quantity updated to 0, then remove it from cart
					if (empty ($quantity)) {
						$this->getQuote()
						->removeItem($itemId);
After, Add

Code: Select all

					//MOD Stop negative quantity values sneaking through
					}
					elseif($quantity < 0) {
						continue;
						
					//MOD END
Post Reply