XSS bug in views/cart/view.json.php

[mb000000]

Hi all,

The astute amongst you will see I'm having the "joys" of PCI compliance testing.... (except, now I've pointed it out, everyone can be astute ;-)  )

Anyway, one thing that has come out of it is a cross site scripting (XSS) problem with Itemid.

In VM2.0.22c (and I've just checked 2.0.26), views/cart/view.json.php contains at around line 50 (in both quoted versions):

Code Select Expand

private function prepareContinueLink() {
// Get a continue link
$menuid = JRequest::getVar('Itemid','');
if(!empty($menuid)){


This should be:

Code Select Expand

private function prepareContinueLink() {
// Get a continue link
$menuid = JRequest::getInt('Itemid','');
if(!empty($menuid)){


Itemid should only ever be numeric, so forcing the get to return a "validated" (by Joomla) int does seem reasonable.

Mark

[AH]

Milbo

Any thoughts on this??

[mb000000]

I see I was a little too early in my proposed fix.

Expanding the original (unchanged) code:

Code Select Expand

private function prepareContinueLink() {
// Get a continue link
$menuid = JRequest::getVar('Itemid','');
if(!empty($menuid)){
$menuid = '&Itemid='.$menuid;
echo '$menuid';
}


I suggest the change actually needs to be:

Code Select Expand

private function prepareContinueLink() {
// Get a continue link
$menuid = JRequest::getInt('Itemid','');
if(!empty($menuid)){
$menuid = '&Itemid='.$menuid;
echo '$menuid';
} else $menuid = '';


Otherwise what happens is that later in the function $menuid is used effectively as a string.  When getVar is used, if it is "empty" then '' (an empty string) is returned, whereas with getInt, it is 0 and so the later string uses append a zero where it then breaks some URIs (oops).

@Hutson - BTW - just about everywhere else in Joomla uses getInt for Itemid, rather than getVar

Sorry about the missed side-effect

Mark

[Milbo]

I wonder why we have the itemId there anyway. Should be done by joomla, no?

[mb000000]

I think Itemid is in there to set the "correct" menu "selector" once the shopper goes back to the category view.  Without it, the menu "selector" will be unknown or potentially random.

Mark

[AH]

This has been updated in vm2.0.26a