Ahh a little WTF to start the morning.

I'm going through some PCI scan results this morning, and in the main it's going well, but I got a couple XSS hits on our catalogsearch pages. This is odd, I think. I've audited these pages, they definitely get routed through magento's escaping code.

On closer examination it turned out the form was okay, it was via the breadcrumbs, that unescaped input was getting into the wild.

I'm running Mage 1.6.x so this code may look a little different if you're running 1.7

Take a look at app/code/core/Mage/CatalogSearch/Block/Result.php, and specifically at the prepareLayout() method:

Now if you look at line 11, if breadcrumbs are enabled, unescaped input is happily added ready for output.

$title = $this->__("Search results for: '%s'", $this->helper('catalogsearch')->getQueryText());

This fix is easy, replace line 10 with:

$title = $this->__("Search results for: '%s'", $this->helper('catalogsearch')->getEscapedQueryText());

This is a really neat example of the evils of duplication and where bad programming practice can lead to real world problems. I am speculating, but it seems reasonable to infer that the original programmer got trigger happy with the copy & paste keys. Later, at some point you could imagine another engineer coming in to XSS safe the code fixed one bit, but (and programmers are human) missed the other (exactly the same line), and we end up with an issue like this.

Personally, I patched the file as described above and stuck it in app/code/local/Mage to override the core code pool version.