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):
private function prepareContinueLink() {
// Get a continue link
$menuid = JRequest::getVar('Itemid','');
if(!empty($menuid)){
This should be:
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
Milbo
Any thoughts on this??
I see I was a little too early in my proposed fix.
Expanding the original (unchanged) code:
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:
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
I wonder why we have the itemId there anyway. Should be done by joomla, no?
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
This has been updated in vm2.0.26a