PCI: Pagination potentially vulnerable to SQL injection, with proposed patch

1

I have a Pod page that requires paging, but a PCI scan reports that the 'pg' parameter used to indicate page number is potentially vulnerable to SQL injection. I don't know if malicious code can in fact be executed, however if you change 'pg' to a string of characters rather than a number, you definitely get a nasty error along the lines of 'your SQL contains errors'.

I determined the ultimate cause to be that page numbering starts at 1 rather than 0, and the simplest solution I then found was to change line 36 of Pod.php from:-

$this->page = empty($this->page) ? 1 : (int) $this->page;

to:-

$this->page = max( (int)$this->page, 1);

This works, because casting any non-number or indeed the empty string to int will result in 0, in which case a value of 1 will be assigned. Likewise any other 'potentially unsafe' values will simply result in the first page being shown.

There's probably a better place to report bugs, but I couldn't find it on the quick, sorry.

asked Oct 1 '10 at 2:35

ttimbul

6

edited Oct 1 '10 at 2:36

add comment
enter at least 15 characters

3 Answers

1

Great fix, will add to our next release. Thanks!

answered Oct 1 '10 at 4:35

sc0ttkclark

2936

add comment
enter at least 15 characters
1

Oh actually, any variable in here is sanitized before that line, but will keep your fix in mind to ensure $this->page ends up being a number.

answered Oct 1 '10 at 4:37

sc0ttkclark

2936

add comment
enter at least 15 characters
1

Committed for 1.9.3

answered Oct 1 '10 at 4:39

sc0ttkclark

2936

add comment
enter at least 15 characters