Opened 5 years ago

Closed 5 years ago

#962 closed Bug (fixed)

Piwik_Common::sanitizeInputValues (and by extension getRequestVar) treats values like "1%6" or "3ab4" as integers

Reported by: mgc8 Owned by:
Priority: normal Milestone: Piwik 0.4.4
Component: Core Keywords: sanitizeInputValues, getRequestVar, sanitize, int, string
Cc: Sensitive: no

Description

The following type of comparison in sanitizeInputValues() is used to ascertain if a string value is actually a string:

if(is_int($value) || $value==(int)$value) $ok = true;

However, the following comparisons are true at least in PHP 5.2.10:

"1%6" == 1

"3ab4" == 3

Apparently the typecasting engine always returns the first "number" part of the string, regardless of the rest; if the first character is not a number, the return will be 0.

I suggest the following modification to solve the issue:

if(is_int($value) || (string)$value==(string)((int)$value)) $ok = true;

This will assure that the comparisons will not be made between a string and an integer directly, thus avoiding the bug.

Change History (4)

comment:1 Changed 5 years ago by vipsoft (robocoder)

  • Milestone set to 1 - Piwik 0.4.4

Since $_GET and $_POST values are strings, don't is_int() and is_float() always fail?

Could we simplify this? Is there a preference in terms of readability and/or performance?

if(is_numeric($value) && is_int((int)$value))  $ok = true;
if((string)$value == (string)(int)$value)  $ok = true;

comment:2 Changed 5 years ago by vipsoft (robocoder)

*scratch my example*

What about this?

if(is_numeric($value) && ($value == (string)(int)$value))  $ok = true;

comment:3 Changed 5 years ago by vipsoft (robocoder)

Ok, the is_numeric() appears to be redundant and a waste of CPU cycles...

comment:4 Changed 5 years ago by vipsoft (robocoder)

  • Resolution set to fixed
  • Status changed from new to closed

In [1452], fix detection of malformed 'integer' and 'float' values

Note: See TracTickets for help on using tickets.