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.)
History
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[]=
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

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.

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