Fork me on GitHub

You are not logged in. Please login or register.

Development

Download

Tickets

Milestones

Subscribe 4

Ticket #633 (fixed bug)

Path disclosure in search.php

  • Created: 2012-02-29 00:53:18
  • Reported by: Franz
  • Assigned to: Oldskool
  • Milestone: 1.4.9
  • Component: code
  • Priority: normal

This code in search.php throws up on arrays (as the input for trim() needs to be a string):

$keywords = (isset($_GET['keywords'])) ? utf8_strtolower(pun_trim($_GET['keywords'])) : null;
$author = (isset($_GET['author'])) ? utf8_strtolower(pun_trim($_GET['author'])) : null;

(E_NOTICE level errors need to be displayed.)

Topic: https://fluxbb.org/forums/viewtopic.php?id=6110

History

Franz 2012-02-29 01:23:13

  • Description changed. (Diff)

daris 2012-02-29 08:37:08

Which code should we use (is_array() or is_string())?

$keywords = (isset($_GET['keywords']) && !is_array($_GET['keywords'])) ? utf8_strtolower(pun_trim($_GET['keywords'])) : null;

or:

$keywords = (isset($_GET['keywords']) && is_string($_GET['keywords'])) ? utf8_strtolower(pun_trim($_GET['keywords'])) : null;

edit: it shows warning for those urls:

http://www.site.com/[path]/search.php?action=search&keywords[]=&author[]=&search_in=all&sort_by=0&SORT_DAshow_as=DESC&topics=&search=Submit+search
http://www.site.com/[path]/userlist.php?username[]=&show_group=-1&sort_by=username&sort_dir=ASC&search=Avvia+ricerca
http://www.site.com/[path]/moderate.php?get_host[]=
Comment edited 2 times (Diff, Diff 2)

Franz 2012-02-29 08:58:57

Use is_string(), I'd say.

daris 2012-03-17 07:44:27

There are lots of such cases where we can place array inside $_GET/$_POST and throw a warning. What do you think about replacing the pun_trim function:

//
// A wrapper for utf8_trim for compatibility
//
function pun_trim($str, $charlist = false)
{
	return utf8_trim($str, $charlist);
}

with the following:

//
// A wrapper for utf8_trim for compatibility
//
function pun_trim($str, $charlist = false)
{
	return is_string($str) ? utf8_trim($str, $charlist) : '';
}

This will prevent from showing a warning for all cases smile

arw 2012-03-25 00:00:37

trim is directly used in some place too :

~/.../s/@ $ grep 'trim(\s*\$_GET' * -rin
admin_bans.php:333:	$expire_after = isset($_GET['expire_after']) ? trim($_GET['expire_after']) : '';
admin_bans.php:334:	$expire_before = isset($_GET['expire_before']) ? trim($_GET['expire_before']) : '';
admin_users.php:126:	$ip = trim($_GET['show_users']);
admin_users.php:683:	$posts_greater = isset($_GET['posts_greater']) ? trim($_GET['posts_greater']) : '';
admin_users.php:684:	$posts_less = isset($_GET['posts_less']) ? trim($_GET['posts_less']) : '';
admin_users.php:685:	$last_post_after = isset($_GET['last_post_after']) ? trim($_GET['last_post_after']) : '';
admin_users.php:686:	$last_post_before = isset($_GET['last_post_before']) ? trim($_GET['last_post_before']) : '';
admin_users.php:687:	$last_visit_after = isset($_GET['last_visit_after']) ? trim($_GET['last_visit_after']) : '';
admin_users.php:688:	$last_visit_before = isset($_GET['last_visit_before']) ? trim($_GET['last_visit_before']) : '';
admin_users.php:689:	$registered_after = isset($_GET['registered_after']) ? trim($_GET['registered_after']) : '';
admin_users.php:690:	$registered_before = isset($_GET['registered_before']) ? trim($_GET['registered_before']) : '';
db_update.php:677:	$uid = trim($_GET['uid']);
extern.php:360:			$fids = explode(',', pun_trim($_GET['fid']));
extern.php:378:			$nfids = explode(',', pun_trim($_GET['nfid']));
search.php:52:		$keywords = (isset($_GET['keywords'])) ? utf8_strtolower(pun_trim($_GET['keywords'])) : null;
search.php:53:		$author = (isset($_GET['author'])) ? utf8_strtolower(pun_trim($_GET['author'])) : null;
search.php:288:				$search_type = array('both', array($keywords, pun_trim($_GET['author'])), implode(',', $forums), $search_in);
search.php:298:				$search_type = array('author', pun_trim($_GET['author']), implode(',', $forums), $search_in);
userlist.php:28:$username = isset($_GET['username']) && $pun_user['g_search_users'] == '1' ? pun_trim($_GET['username']) : '';
~/.../s/@ $ grep 'trim(\s*\$_POST' * -rin
admin_bans.php:43:			$ban_user = pun_trim($_POST['new_ban_user']);
admin_bans.php:185:	$ban_user = pun_trim($_POST['ban_user']);
admin_bans.php:186:	$ban_ip = trim($_POST['ban_ip']);
admin_bans.php:187:	$ban_email = strtolower(trim($_POST['ban_email']));
admin_bans.php:188:	$ban_message = pun_trim($_POST['ban_message']);
admin_bans.php:189:	$ban_expire = trim($_POST['ban_expire']);
admin_categories.php:28:	$new_cat_name = pun_trim($_POST['new_cat_name']);
admin_censoring.php:28:	$search_for = pun_trim($_POST['new_search_for']);
admin_censoring.php:29:	$replace_with = pun_trim($_POST['new_replace_with']);
admin_censoring.php:52:	$search_for = pun_trim($_POST['search_for'][$id]);
admin_censoring.php:53:	$replace_with = pun_trim($_POST['replace_with'][$id]);
admin_forums.php:158:		$forum_name = pun_trim($_POST['forum_name']);
admin_forums.php:159:		$forum_desc = pun_linebreaks(pun_trim($_POST['forum_desc']));
admin_forums.php:162:		$redirect_url = isset($_POST['redirect_url']) ? trim($_POST['redirect_url']) : null;
admin_groups.php:252:	$title = pun_trim($_POST['req_title']);
admin_groups.php:253:	$user_title = pun_trim($_POST['user_title']);
admin_maintenance.php:128:	$prune_from = trim($_POST['prune_from']);
admin_maintenance.php:175:	$prune_days = trim($_POST['req_prune_days']);
admin_options.php:28:		'board_title'			=> pun_trim($_POST['form']['board_title']),
admin_options.php:29:		'board_desc'			=> pun_trim($_POST['form']['board_desc']),
admin_options.php:30:		'base_url'				=> pun_trim($_POST['form']['base_url']),
admin_options.php:33:		'default_lang'			=> pun_trim($_POST['form']['default_lang']),
admin_options.php:34:		'default_style'			=> pun_trim($_POST['form']['default_style']),
admin_options.php:35:		'time_format'			=> pun_trim($_POST['form']['time_format']),
admin_options.php:36:		'date_format'			=> pun_trim($_POST['form']['date_format']),
admin_options.php:61:		'additional_navlinks'	=> pun_trim($_POST['form']['additional_navlinks']),
admin_options.php:65:		'mailing_list'			=> pun_trim($_POST['form']['mailing_list']),
admin_options.php:67:		'avatars_dir'			=> pun_trim($_POST['form']['avatars_dir']),
admin_options.php:71:		'admin_email'			=> strtolower(pun_trim($_POST['form']['admin_email'])),
admin_options.php:72:		'webmaster_email'		=> strtolower(pun_trim($_POST['form']['webmaster_email'])),
admin_options.php:75:		'smtp_host'				=> pun_trim($_POST['form']['smtp_host']),
admin_options.php:76:		'smtp_user'				=> pun_trim($_POST['form']['smtp_user']),
admin_options.php:82:		'rules_message'			=> pun_trim($_POST['form']['rules_message']),
admin_options.php:85:		'announcement_message'	=> pun_trim($_POST['form']['announcement_message']),
admin_options.php:87:		'maintenance_message'	=> pun_trim($_POST['form']['maintenance_message']),
admin_options.php:133:		$smtp_pass1 = isset($_POST['form']['smtp_pass1']) ? pun_trim($_POST['form']['smtp_pass1']) : '';
admin_options.php:134:		$smtp_pass2 = isset($_POST['form']['smtp_pass2']) ? pun_trim($_POST['form']['smtp_pass2']) : '';
admin_ranks.php:28:	$rank = pun_trim($_POST['new_rank']);
admin_ranks.php:29:	$min_posts = trim($_POST['new_min_posts']);
admin_ranks.php:61:	$rank = pun_trim($_POST['rank'][$id]);
admin_ranks.php:62:	$min_posts = trim($_POST['min_posts'][$id]);
admin_users.php:565:		$ban_message = pun_trim($_POST['ban_message']);
admin_users.php:566:		$ban_expire = pun_trim($_POST['ban_expire']);
db_update.php:624:	$req_db_pass = strtolower(trim($_POST['req_db_pass']));
db_update.php:1530:				$username = pun_trim($_POST['dupe_users'][$id]);
edit.php:62:		$subject = pun_trim($_POST['req_subject']);
edit.php:78:	$message = pun_linebreaks(pun_trim($_POST['req_message']));
login.php:23:	$form_username = pun_trim($_POST['req_username']);
login.php:24:	$form_password = pun_trim($_POST['req_password']);
login.php:125:		$email = strtolower(trim($_POST['req_email']));
misc.php:103:		$subject = pun_trim($_POST['req_subject']);
misc.php:104:		$message = pun_trim($_POST['req_message']);
misc.php:207:		$reason = pun_linebreaks(pun_trim($_POST['req_reason']));
moderate.php:180:			$new_subject = isset($_POST['new_subject']) ? pun_trim($_POST['new_subject']) : '';
post.php:69:		$subject = pun_trim($_POST['req_subject']);
post.php:93:		$username = pun_trim($_POST['req_username']);
post.php:123:	$orig_message = $message = pun_linebreaks(pun_trim($_POST['req_message']));
profile.php:87:		$old_password = isset($_POST['req_old_password']) ? pun_trim($_POST['req_old_password']) : '';
profile.php:88:		$new_password1 = pun_trim($_POST['req_new_password1']);
profile.php:89:		$new_password2 = pun_trim($_POST['req_new_password2']);
profile.php:202:		$new_email = strtolower(trim($_POST['req_new_email']));
profile.php:715:				$form['language'] = pun_trim($_POST['form']['language']);
profile.php:722:				$form['admin_note'] = pun_trim($_POST['admin_note']);
profile.php:727:					$form['username'] = pun_trim($_POST['req_username']);
profile.php:753:				$form['email'] = strtolower(trim($_POST['req_email']));
profile.php:764:				'realname'		=> pun_trim($_POST['form']['realname']),
profile.php:765:				'url'			=> pun_trim($_POST['form']['url']),
profile.php:766:				'location'		=> pun_trim($_POST['form']['location']),
profile.php:781:				$form['title'] = pun_trim($_POST['title']);
profile.php:784:				$form['title'] = pun_trim($_POST['title']);
profile.php:803:				'jabber'		=> pun_trim($_POST['form']['jabber']),
profile.php:804:				'icq'			=> pun_trim($_POST['form']['icq']),
profile.php:805:				'msn'			=> pun_trim($_POST['form']['msn']),
profile.php:806:				'aim'			=> pun_trim($_POST['form']['aim']),
profile.php:807:				'yahoo'			=> pun_trim($_POST['form']['yahoo']),
profile.php:824:				$form['signature'] = pun_linebreaks(pun_trim($_POST['signature']));
profile.php:854:				'disp_topics'		=> pun_trim($_POST['form']['disp_topics']),
profile.php:855:				'disp_posts'		=> pun_trim($_POST['form']['disp_posts']),
profile.php:885:				$form['style'] = pun_trim($_POST['form']['style']);
register.php:75:	$username = pun_trim($_POST['req_user']);
register.php:76:	$email1 = strtolower(trim($_POST['req_email1']));
register.php:80:		$email2 = strtolower(trim($_POST['req_email2']));
register.php:87:		$password1 = pun_trim($_POST['req_password1']);
register.php:88:		$password2 = pun_trim($_POST['req_password2']);

Franz 2012-03-27 11:13:19

The pun_trim() improvement is very good, I'd say. We should probably just use pun_trim() instead of trim() for the sake of simplicity and consistency in the cases arw listed.

arw 2012-04-02 21:25:27

i added that to a pull request :
https://github.com/fluxbb/fluxbb/pull/34

Oldskool 2012-04-02 21:33:11

Commit 3e5349e to fluxbb fluxbb-1.4

Merge pull request #36 from Etana/fluxbb-1.4#633

commit for #633 ( trim on array issue )

Oldskool 2012-04-02 21:42:14

  • Status changed from open to fixed.

Thanks Etana (Arw) for your commit!

Franz 2012-04-24 13:14:47

  • Owner set to Oldskool.

Franz 2012-05-08 10:18:55

Oversights? https://fluxbb.org/forums/viewtopic.php?id=6335

arw 2012-05-08 11:41:23

you said " in the cases arw listed " so i only replaced it in the case i listed ( so when arguments was coming from globals $_GET or $_POST )

if it's wanted to replace every trim, there is still all these : http://codepad.org/M6Ink99x