Ciąg dalszy problemów z przekazywaniem tablic w phpBB by Przemo 1.12.5

Jeszcze w wigilię roku poprzedniego opisywałem rozwiązanie drobnego problemu z przekazywaniem tablic w jednym z plików forum phpBB by Przemo. Co ciekawsze na zasadzie tak jak poprzednio doszliśmy do wniosku, że skrypt ten w większości przypadków filtrowania danych wejściowych nie pozwala na poprawne filtrowanie przekazywanych tablic.

Dla przykładu podam kilka konkretnych błędnych elementów kodu wraz z jednym z rozwiązań poprawkowych. Dodam na wstępie, że wszelkie podane bugi dotyczą elementu tablicy globalnej o nazwie mode lub folder występujących w kilku osobnych plikach. Kolejno – privmsg.php standardowo może przyjmować domyślnie w tym elemencie wartości takie jak birthday, newpm, czy read. Co jeśli ktoś zażyczy sobie np. dla testu przekazać wartość w postaci elementu tablicy?

if ( !empty($HTTP_POST_VARS['mode']) || !empty($HTTP_GET_VARS['mode']) )
{
	$mode = ( !empty($HTTP_POST_VARS['mode']) ) ? $HTTP_POST_VARS['mode'] : $HTTP_GET_VARS['mode'];
	$mode = htmlspecialchars($mode);
}
else
{
	$mode = '';
}

To kod odpowiadający za podstawowe filtrowanie wprowadzanych danych. Niestety nie sprawdza się on w wypadku, gdy w adresie forum wprowadzimy np. poniższy adres:

/privmsg.php?mode[]=1,2,3

Niby w tym przykładzie oprócz wizualnych większych szkód nie ma, nie mniej jest to bug. Teoretycznie można by wprowadzić dwa rodzaje korekt w tym kodzie. Jeden w stylu mojego ostatniego wpisu z tej tematyki, a mianowicie:

if ( is_array($HTTP_POST_VARS['mode']) || is_array($HTTP_GET_VARS['mode']) )
{
	$mode = '';
}
else if ( !empty($HTTP_POST_VARS['mode']) || !empty($HTTP_GET_VARS['mode']) )
{
	$mode = ( !empty($HTTP_POST_VARS['mode']) ) ? $HTTP_POST_VARS['mode'] : $HTTP_GET_VARS['mode'];
	$mode = htmlspecialchars($mode);
}
else
{
	$mode = '';
}

Dodałem w tym wypadku po prostu pierwszy warunek który sprawdza czy przesyłane przez $_GET lub $_POST wartości nie są tablicą, a jeżeli są to zmienna $mode staje się pusta. Jest to o tyle bezpieczna wersja poprawki, że wyklucza przekazywanie jakichkolwiek tablic w tym przypadku. Druga wersja poprawki to taka, która pozwala na przekazywanie tablic, ale filtruje je w taki sposób, aby odebrane przez skrypt były jako string.

if ( is_array($HTTP_POST_VARS['mode']) || is_array($HTTP_GET_VARS['mode']) )
{
	$mode = ( !empty($HTTP_POST_VARS['mode']) ) ? $HTTP_POST_VARS['mode'] : $HTTP_GET_VARS['mode'];
	$mode = htmlspecialchars($mode[0]);
}
else if ( !empty($HTTP_POST_VARS['mode']) || !empty($HTTP_GET_VARS['mode']) )
{
	$mode = ( !empty($HTTP_POST_VARS['mode']) ) ? $HTTP_POST_VARS['mode'] : $HTTP_GET_VARS['mode'];
	$mode = htmlspecialchars($mode);
}
else
{
	$mode = '';
}

Adekwatnie sprawa wygląda w plikach profile.php i modcp.php, a także w privmsg.php w związku z elementem folder. Ciut inna sytuacja występuje za to w posting.php. Tam filtrowane są w pętli wszystkie niezbędne elementy tablic.

$params = array('submit' => 'post', 'preview' => 'preview', 'delete' => 'delete', 'poll_delete' => 'poll_delete', 'poll_add' => 'add_poll_option', 'poll_edit' => 'edit_poll_option', 'mode' => 'mode');
while( list($var, $param) = @each($params) )
{
	if ( !empty($HTTP_POST_VARS[$param]) || !empty($HTTP_GET_VARS[$param]) )
	{
		$$var = ( !empty($HTTP_POST_VARS[$param]) ) ? htmlspecialchars($HTTP_POST_VARS[$param]) : htmlspecialchars($HTTP_GET_VARS[$param]);
	}
	else
	{
		$$var = '';
	}
}

Co możemy zamienić tak jak w pierwszym przypadku na:

//
// Check and set various parameters
//
$params = array('submit' => 'post', 'preview' => 'preview', 'delete' => 'delete', 'poll_delete' => 'poll_delete', 'poll_add' => 'add_poll_option', 'poll_edit' => 'edit_poll_option', 'mode' => 'mode');
while( list($var, $param) = @each($params) )
{
	if ( is_array($HTTP_POST_VARS[$param]) || is_array($HTTP_GET_VARS[$param]) )
	{
		$$var = '';
	}
	else if ( !empty($HTTP_POST_VARS[$param]) || !empty($HTTP_GET_VARS[$param]) )
	{
		$$var = ( !empty($HTTP_POST_VARS[$param]) ) ? htmlspecialchars($HTTP_POST_VARS[$param]) : htmlspecialchars($HTTP_GET_VARS[$param]);
	}
	else
	{
		$$var = '';
	}
}

lub

//
// Check and set various parameters
//
$params = array('submit' => 'post', 'preview' => 'preview', 'delete' => 'delete', 'poll_delete' => 'poll_delete', 'poll_add' => 'add_poll_option', 'poll_edit' => 'edit_poll_option', 'mode' => 'mode');
while( list($var, $param) = @each($params) )
{
	if ( is_array($HTTP_POST_VARS[$param]) || is_array($HTTP_GET_VARS[$param]) )
	{
		$$var = ( !empty($HTTP_POST_VARS[$param]) ) ?  htmlspecialchars($HTTP_POST_VARS[$param][0]) :  htmlspecialchars($HTTP_GET_VARS[$param][0]);
	}
	else if ( !empty($HTTP_POST_VARS[$param]) || !empty($HTTP_GET_VARS[$param]) )
	{
		$$var = ( !empty($HTTP_POST_VARS[$param]) ) ? htmlspecialchars($HTTP_POST_VARS[$param]) : htmlspecialchars($HTTP_GET_VARS[$param]);
	}
	else
	{
		$$var = '';
	}
}

Są to tylko przykłady błędów. Reasumując całą sytuację należałoby względnie tych przykładów pozmieniać filtrując w taki sposób wszelkie zmienne wejściowe (z tablic superglobalnych $_GET, $_POST) przy których jesteśmy pewni, że nie będziemy przekazywać w nich żadnych wartości tablicowych. Uchroni nas to od błędów typu:

Warning: htmlspecialchars() expects parameter 1 to be string, array given in /adres/modcp.php on line 116