From c695136e4d75695178a9fc848a7cf6bfa2b9346c Mon Sep 17 00:00:00 2001 From: plegall Date: Fri, 19 Mar 2010 22:25:39 +0000 Subject: bug 1328: backport the pwg_token on trunk bug 1329: backport the check_input_parameter on trunk feature 1026: add pwg_token feature for edit/delete comment. Heavy refactoring on this feature to make the code simpler and easier to maintain (I hope). git-svn-id: http://piwigo.org/svn/trunk@5195 68402e56-0260-453c-a942-63ccdbb3a9ee --- admin/cat_list.php | 9 ++ admin/element_set.php | 2 + admin/element_set_global.php | 5 + admin/group_list.php | 10 +- admin/include/functions.php | 28 ---- admin/include/uploadify/uploadify.php | 2 +- admin/photos_add_direct.php | 4 +- admin/picture_modify.php | 3 + admin/plugins_list.php | 9 +- admin/plugins_new.php | 6 +- admin/plugins_update.php | 7 +- admin/site_manager.php | 18 ++- admin/tags.php | 8 +- admin/themes/default/template/cat_list.tpl | 2 + admin/themes/default/template/group_list.tpl | 1 + admin/themes/default/template/site_manager.tpl | 2 + admin/themes/default/template/tags.tpl | 1 + comments.php | 196 +++++++++++++++++-------- feed.php | 2 +- include/constants.php | 3 + include/functions.inc.php | 82 +++++++++++ include/functions_comment.inc.php | 55 +++++-- include/functions_user.inc.php | 39 ++++- include/picture_comment.inc.php | 44 +++--- picture.php | 61 +++++--- search.php | 4 +- 26 files changed, 433 insertions(+), 170 deletions(-) diff --git a/admin/cat_list.php b/admin/cat_list.php index 7168b3fd0..5aadeb3c2 100644 --- a/admin/cat_list.php +++ b/admin/cat_list.php @@ -33,6 +33,11 @@ include_once(PHPWG_ROOT_PATH.'admin/include/functions.php'); // +-----------------------------------------------------------------------+ check_status(ACCESS_ADMINISTRATOR); +if (!empty($_POST) or isset($_GET['delete'])) +{ + check_pwg_token(); +} + // +-----------------------------------------------------------------------+ // | functions | // +-----------------------------------------------------------------------+ @@ -64,6 +69,8 @@ function save_categories_order($categories) // | initialization | // +-----------------------------------------------------------------------+ +check_input_parameter('parent_id', $_GET, false, PATTERN_ID); + $categories = array(); $base_url = get_root_url().'admin.php?page=cat_list'; @@ -185,6 +192,7 @@ if (isset($_GET['parent_id'])) $template->assign(array( 'CATEGORIES_NAV'=>$navigation, 'F_ACTION'=>$form_action, + 'PWG_TOKEN' => get_pwg_token(), )); // +-----------------------------------------------------------------------+ @@ -260,6 +268,7 @@ foreach ($categories as $category) if (empty($category['dir'])) { $tpl_cat['U_DELETE'] = $self_url.'&delete='.$category['id']; + $tpl_cat['U_DELETE'].= '&pwg_token='.get_pwg_token(); } if ( array_key_exists($category['id'], $categories_with_images) ) diff --git a/admin/element_set.php b/admin/element_set.php index 49b1b2377..1b5e88719 100644 --- a/admin/element_set.php +++ b/admin/element_set.php @@ -39,6 +39,8 @@ include_once(PHPWG_ROOT_PATH.'admin/include/functions.php'); // +-----------------------------------------------------------------------+ check_status(ACCESS_ADMINISTRATOR); +check_input_parameter('selection', $_POST, true, PATTERN_ID); + // +-----------------------------------------------------------------------+ // | caddie management | // +-----------------------------------------------------------------------+ diff --git a/admin/element_set_global.php b/admin/element_set_global.php index 113b5b1f8..73cc720b0 100644 --- a/admin/element_set_global.php +++ b/admin/element_set_global.php @@ -43,6 +43,11 @@ check_status(ACCESS_ADMINISTRATOR); // | deletion form submission | // +-----------------------------------------------------------------------+ +// the $_POST['selection'] was already checked in element_set.php +check_input_parameter('del_tags', $_POST, true, PATTERN_ID); +check_input_parameter('associate', $_POST, false, PATTERN_ID); +check_input_parameter('dissociate', $_POST, false, PATTERN_ID); + if (isset($_POST['delete'])) { if (isset($_POST['confirm_deletion']) and 1 == $_POST['confirm_deletion']) diff --git a/admin/group_list.php b/admin/group_list.php index 416d78bb9..7c42d9613 100644 --- a/admin/group_list.php +++ b/admin/group_list.php @@ -33,6 +33,11 @@ include_once(PHPWG_ROOT_PATH.'admin/include/functions.php'); // +-----------------------------------------------------------------------+ check_status(ACCESS_ADMINISTRATOR); +if (!empty($_POST) or isset($_GET['delete']) or isset($_GET['toggle_is_default'])) +{ + check_pwg_token(); +} + // +-----------------------------------------------------------------------+ // | delete a group | // +-----------------------------------------------------------------------+ @@ -155,6 +160,7 @@ $template->assign( array( 'F_ADD_ACTION' => get_root_url().'admin.php?page=group_list', 'U_HELP' => get_root_url().'popuphelp.php?page=group_list', + 'PWG_TOKEN' => get_pwg_token(), ) ); @@ -191,9 +197,9 @@ SELECT COUNT(*) 'IS_DEFAULT' => (get_boolean($row['is_default']) ? ' ['.l10n('default').']' : ''), 'MEMBERS' => l10n_dec('%d member', '%d members', $counter), 'U_MEMBERS' => $members_url.$row['id'], - 'U_DELETE' => $del_url.$row['id'], + 'U_DELETE' => $del_url.$row['id'].'&pwg_token='.get_pwg_token(), 'U_PERM' => $perm_url.$row['id'], - 'U_ISDEFAULT' => $toggle_is_default_url.$row['id'] + 'U_ISDEFAULT' => $toggle_is_default_url.$row['id'].'&pwg_token='.get_pwg_token(), ) ); } diff --git a/admin/include/functions.php b/admin/include/functions.php index c7b8bf5f1..e3522702f 100644 --- a/admin/include/functions.php +++ b/admin/include/functions.php @@ -23,34 +23,6 @@ include(PHPWG_ROOT_PATH.'admin/include/functions_metadata.php'); -/** - * check token comming from form posted or get params to prevent csrf attacks - * if pwg_token is empty action doesn't require token - * else pwg_token is compare to server token - * - * @return void access denied if token given is not equal to server token - */ -function check_token() -{ - global $conf; - - $valid_token = hash_hmac('md5', session_id(), $conf['secret_key']); - $given_token = null; - - if (!empty($_POST['pwg_token'])) - { - $given_token = $_POST['pwg_token']; - } - elseif (!empty($_GET['pwg_token'])) - { - $given_token = $_GET['pwg_token']; - } - if ($given_token != $valid_token) - { - access_denied(); - } -} - // The function delete_site deletes a site and call the function // delete_categories for each primary category of the site function delete_site( $id ) diff --git a/admin/include/uploadify/uploadify.php b/admin/include/uploadify/uploadify.php index eddcc12b0..306b492da 100644 --- a/admin/include/uploadify/uploadify.php +++ b/admin/include/uploadify/uploadify.php @@ -8,7 +8,7 @@ include_once(PHPWG_ROOT_PATH.'include/common.inc.php'); include_once(PHPWG_ROOT_PATH.'admin/include/functions.php'); include_once(PHPWG_ROOT_PATH.'admin/include/functions_upload.inc.php'); -// check_pwg_token(); +check_pwg_token(); ob_start(); print_r($_FILES); diff --git a/admin/photos_add_direct.php b/admin/photos_add_direct.php index 4fceb6b12..0c24fc6ef 100644 --- a/admin/photos_add_direct.php +++ b/admin/photos_add_direct.php @@ -30,7 +30,7 @@ if (!defined('PHOTOS_ADD_BASE_URL')) if (isset($_GET['batch'])) { - check_input_parameter('batch', $_GET['batch'], false, '/^\d+(,\d+)*$/'); + check_input_parameter('batch', $_GET, false, '/^\d+(,\d+)*$/'); $query = ' DELETE FROM '.CADDIE_TABLE.' @@ -347,7 +347,7 @@ $template->assign( 'switch_url' => PHOTOS_ADD_BASE_URL.'&upload_mode='.$upload_switch, 'upload_id' => md5(rand()), 'session_id' => session_id(), - 'pwg_token' => '1234abcd5678efgh',// get_pwg_token(), + 'pwg_token' => get_pwg_token(), ) ); diff --git a/admin/picture_modify.php b/admin/picture_modify.php index e33447f70..e61a324e7 100644 --- a/admin/picture_modify.php +++ b/admin/picture_modify.php @@ -33,6 +33,9 @@ include_once(PHPWG_ROOT_PATH.'admin/include/functions.php'); // +-----------------------------------------------------------------------+ check_status(ACCESS_ADMINISTRATOR); +check_input_parameter('image_id', $_GET, false, PATTERN_ID); +check_input_parameter('cat_id', $_GET, false, PATTERN_ID); + // +-----------------------------------------------------------------------+ // | synchronize metadata | // +-----------------------------------------------------------------------+ diff --git a/admin/plugins_list.php b/admin/plugins_list.php index 2b12f171c..2f0eab1b2 100644 --- a/admin/plugins_list.php +++ b/admin/plugins_list.php @@ -32,12 +32,15 @@ $template->set_filenames(array('plugins' => 'plugins_list.tpl')); $order = isset($_GET['order']) ? $_GET['order'] : 'name'; $base_url = get_root_url().'admin.php?page='.$page['page'].'&order='.$order; +$action_url = $base_url.'&plugin='.'%s'.'&pwg_token='.get_pwg_token(); $plugins = new plugins(); //--------------------------------------------------perform requested actions if (isset($_GET['action']) and isset($_GET['plugin']) and !is_adviser()) { + check_pwg_token(); + $page['errors'] = $plugins->perform_action($_GET['action'], $_GET['plugin']); if (empty($page['errors'])) @@ -96,7 +99,7 @@ foreach($plugins->fs_plugins as $plugin_id => $fs_plugin) array('NAME' => $display_name, 'VERSION' => $fs_plugin['version'], 'DESCRIPTION' => $desc, - 'U_ACTION' => $base_url.'&plugin='.$plugin_id); + 'U_ACTION' => sprintf($action_url, $plugin_id)); if (isset($plugins->db_plugins_by_id[$plugin_id])) { @@ -115,14 +118,12 @@ $missing_plugin_ids = array_diff( foreach($missing_plugin_ids as $plugin_id) { - $action_url = $base_url.'&plugin='.$plugin_id; - $template->append( 'plugins', array( 'NAME' => $plugin_id, 'VERSION' => $plugins->db_plugins_by_id[$plugin_id]['version'], 'DESCRIPTION' => "ERROR: THIS PLUGIN IS MISSING BUT IT IS INSTALLED! UNINSTALL IT NOW !", - 'U_ACTION' => $base_url.'&plugin='.$plugin_id, + 'U_ACTION' => sprintf($action_url, $plugin_id), 'STATE' => 'missing' ) ); diff --git a/admin/plugins_new.php b/admin/plugins_new.php index 8ee9d26ed..a9b15c06b 100644 --- a/admin/plugins_new.php +++ b/admin/plugins_new.php @@ -38,6 +38,8 @@ $plugins = new plugins(); //------------------------------------------------------automatic installation if (isset($_GET['revision']) and isset($_GET['extension']) and !is_adviser()) { + check_pwg_token(); + $install_status = $plugins->extract_plugin_files('install', $_GET['revision'], $_GET['extension']); redirect($base_url.'&installstatus='.$install_status); @@ -110,7 +112,9 @@ if ($plugins->get_server_plugins(true)) $url_auto_install = htmlentities($base_url) . '&revision=' . $plugin['revision_id'] - . '&extension=' . $plugin['extension_id']; + . '&extension=' . $plugin['extension_id'] + . '&pwg_token='.get_pwg_token() + ; $template->append('plugins', array( 'EXT_NAME' => $plugin['extension_name'], diff --git a/admin/plugins_update.php b/admin/plugins_update.php index 4566d8fd9..af8e527cb 100644 --- a/admin/plugins_update.php +++ b/admin/plugins_update.php @@ -37,6 +37,8 @@ $plugins = new plugins(); //-----------------------------------------------------------automatic upgrade if (isset($_GET['plugin']) and isset($_GET['revision']) and !is_adviser()) { + check_pwg_token(); + $plugin_id = $_GET['plugin']; $revision = $_GET['revision']; @@ -48,6 +50,7 @@ if (isset($_GET['plugin']) and isset($_GET['revision']) and !is_adviser()) redirect($base_url . '&revision=' . $revision . '&plugin=' . $plugin_id + . '&pwg_token='.get_pwg_token() . '&reactivate=true'); } @@ -133,7 +136,9 @@ if ($plugins->get_server_plugins()) // Plugin need upgrade $url_auto_update = $base_url . '&revision=' . $plugin_info['revision_id'] - . '&plugin=' . $plugin_id; + . '&plugin=' . $plugin_id + . '&pwg_token='.get_pwg_token() + ; $template->append('plugins_not_uptodate', array( 'EXT_NAME' => $fs_plugin['name'], diff --git a/admin/site_manager.php b/admin/site_manager.php index ee25bd826..595605f59 100644 --- a/admin/site_manager.php +++ b/admin/site_manager.php @@ -33,6 +33,11 @@ include_once(PHPWG_ROOT_PATH.'admin/include/functions.php'); // +-----------------------------------------------------------------------+ check_status(ACCESS_ADMINISTRATOR); +if (!empty($_POST) or isset($_GET['action'])) +{ + check_pwg_token(); +} + /** * requests the given $url (a remote create_listing_file.php) and fills a * list of lines corresponding to request output @@ -198,11 +203,13 @@ SELECT galleries_url } } -$template->assign( array( - 'U_HELP' => get_root_url().'popuphelp.php?page=site_manager', - 'F_ACTION' => get_root_url().'admin.php' - .get_query_string_diff( array('action','site') ) - ) ); +$template->assign( + array( + 'U_HELP' => get_root_url().'popuphelp.php?page=site_manager', + 'F_ACTION' => get_root_url().'admin.php'.get_query_string_diff(array('action','site','pwg_token')), + 'PWG_TOKEN' => get_pwg_token(), + ) + ); // +-----------------------------------------------------------------------+ // | remote sites list | @@ -242,6 +249,7 @@ while ($row = pwg_db_fetch_assoc($result)) $base_url = PHPWG_ROOT_PATH.'admin.php'; $base_url.= '?page=site_manager'; $base_url.= '&site='.$row['id']; + $base_url.= '&pwg_token='.get_pwg_token(); $base_url.= '&action='; $update_url = PHPWG_ROOT_PATH.'admin.php'; diff --git a/admin/tags.php b/admin/tags.php index 24368bcfb..caade0058 100644 --- a/admin/tags.php +++ b/admin/tags.php @@ -29,6 +29,11 @@ if( !defined("PHPWG_ROOT_PATH") ) include_once(PHPWG_ROOT_PATH.'admin/include/functions.php'); check_status(ACCESS_ADMINISTRATOR); +if (!empty($_POST)) +{ + check_pwg_token(); +} + // +-----------------------------------------------------------------------+ // | edit tags | // +-----------------------------------------------------------------------+ @@ -189,7 +194,8 @@ $template->set_filenames(array('tags' => 'tags.tpl')); $template->assign( array( - 'F_ACTION' => PHPWG_ROOT_PATH.'admin.php?page=tags' + 'F_ACTION' => PHPWG_ROOT_PATH.'admin.php?page=tags', + 'PWG_TOKEN' => get_pwg_token(), ) ); diff --git a/admin/themes/default/template/cat_list.tpl b/admin/themes/default/template/cat_list.tpl index 431845d2a..e80558c9e 100644 --- a/admin/themes/default/template/cat_list.tpl +++ b/admin/themes/default/template/cat_list.tpl @@ -26,6 +26,7 @@

{$CATEGORIES_NAV}

+

{'Add a virtual category'|@translate} : @@ -38,6 +39,7 @@ {if count($categories) } +

diff --git a/admin/themes/default/template/group_list.tpl b/admin/themes/default/template/group_list.tpl index 6b32da66b..ab74985a4 100644 --- a/admin/themes/default/template/group_list.tpl +++ b/admin/themes/default/template/group_list.tpl @@ -3,6 +3,7 @@ +

{'Add group'|@translate} diff --git a/admin/themes/default/template/site_manager.tpl b/admin/themes/default/template/site_manager.tpl index 91d888ac0..6dce1fec0 100644 --- a/admin/themes/default/template/site_manager.tpl +++ b/admin/themes/default/template/site_manager.tpl @@ -16,6 +16,7 @@ {'A local listing.xml file has been found for '|@translate} {$local_listing.URL} {if isset($local_listing.CREATE)} +

{'Create this site'|@translate}: @@ -63,6 +64,7 @@ {/if} +

diff --git a/admin/themes/default/template/tags.tpl b/admin/themes/default/template/tags.tpl index 3db8417a6..4d7031897 100644 --- a/admin/themes/default/template/tags.tpl +++ b/admin/themes/default/template/tags.tpl @@ -3,6 +3,7 @@ + {if isset($EDIT_TAGS_LIST)}

diff --git a/comments.php b/comments.php index c020a84ce..8127bd191 100644 --- a/comments.php +++ b/comments.php @@ -116,6 +116,26 @@ if (!empty($_GET['author'])) OR author = \''.$_GET['author'].'\''; } +// search a specific comment (if you're coming directly from an admin +// notification email) +if (!empty($_GET['comment_id'])) +{ + check_input_parameter('comment_id', $_GET, false, PATTERN_ID); + + // currently, the $_GET['comment_id'] is only used by admins from email + // for management purpose (validate/delete) + if (!is_admin()) + { + $login_url = + get_root_url().'identification.php?redirect=' + .urlencode(urlencode($_SERVER['REQUEST_URI'])) + ; + redirect($login_url); + } + + $page['where_clauses'][] = 'com.id = '.$_GET['comment_id']; +} + // search a substring among comments content if (!empty($_GET['keyword'])) { @@ -155,41 +175,74 @@ $page['where_clauses'][] = get_sql_condition_FandF // +-----------------------------------------------------------------------+ // | comments management | // +-----------------------------------------------------------------------+ -if (isset($_GET['delete']) and is_numeric($_GET['delete']) - and (is_admin() || $conf['user_can_delete_comment'])) -{// comments deletion - delete_user_comment($_GET['delete']); -} -if (isset($_GET['validate']) and is_numeric($_GET['validate']) - and !is_adviser() ) -{ // comments validation - check_status(ACCESS_ADMINISTRATOR); - $query = ' -UPDATE '.COMMENTS_TABLE.' - SET validated = \'true\' - , validation_date = NOW() - WHERE id='.$_GET['validate'].' -;'; - pwg_query($query); -} +$comment_id = null; +$action = null; -if (isset($_GET['edit']) and is_numeric($_GET['edit']) - and (is_admin() || $conf['user_can_edit_comment'])) +$actions = array('delete', 'validate', 'edit'); +foreach ($actions as $loop_action) { - if (!empty($_POST['content'])) + if (isset($_GET[$loop_action])) { - update_user_comment(array('comment_id' => $_GET['edit'], - 'image_id' => $_POST['image_id'], - 'content' => $_POST['content']), - $_POST['key'] - ); - - $edit_comment = null; + $action = $loop_action; + check_input_parameter($action, $_GET, false, PATTERN_ID); + $comment_id = $_GET[$action]; + break; } - else +} + +if (isset($action)) +{ + check_pwg_token(); + + $comment_author_id = get_comment_author_id($comment_id); + + if (can_manage_comment($action, $comment_author_id)) { - $edit_comment = $_GET['edit']; + $perform_redirect = false; + + if ('delete' == $action) + { + delete_user_comment($comment_id); + $perform_redirect = true; + } + + if ('validate' == $action) + { + validate_user_comment($comment_id); + $perform_redirect = true; + } + + if ('edit' == $action) + { + if (!empty($_POST['content'])) + { + update_user_comment( + array( + 'comment_id' => $_GET['edit'], + 'image_id' => $_POST['image_id'], + 'content' => $_POST['content'] + ), + $_POST['key'] + ); + + $edit_comment = null; + } + else + { + $edit_comment = $_GET['edit']; + } + } + + if ($perform_redirect) + { + $redirect_url = + PHPWG_ROOT_PATH + .'comments.php' + .get_query_string_diff(array('delete','validate','pwg_token')); + + redirect($redirect_url); + } } } @@ -287,7 +340,7 @@ list($counter) = pwg_db_fetch_row(pwg_query($query)); $url = PHPWG_ROOT_PATH .'comments.php' - .get_query_string_diff(array('start','delete','validate')); + .get_query_string_diff(array('start','delete','validate','pwg_token')); $navbar = create_navigation_bar($url, $counter, @@ -378,40 +431,53 @@ SELECT id, name, permalink, uppercats // link to the full size picture $url = make_picture_url( - array( - 'category' => $categories[ $comment['category_id'] ], - 'image_id' => $comment['image_id'], - 'image_file' => $elements[$comment['image_id']]['file'], - ) - ); - - $tpl_comment = array( - 'U_PICTURE' => $url, - 'TN_SRC' => $thumbnail_src, - 'ALT' => $name, - 'AUTHOR' => trigger_event('render_comment_author', $comment['author']), - 'DATE'=>format_date($comment['date'], true), - 'CONTENT'=>trigger_event('render_comment_content',$comment['content']), - ); + 'category' => $categories[ $comment['category_id'] ], + 'image_id' => $comment['image_id'], + 'image_file' => $elements[$comment['image_id']]['file'], + ) + ); + + $tpl_comment = array( + 'U_PICTURE' => $url, + 'TN_SRC' => $thumbnail_src, + 'ALT' => $name, + 'AUTHOR' => trigger_event('render_comment_author', $comment['author']), + 'DATE'=>format_date($comment['date'], true), + 'CONTENT'=>trigger_event('render_comment_content',$comment['content']), + ); if (can_manage_comment('delete', $comment['author_id'])) { - $url = get_root_url().'comments.php' - .get_query_string_diff(array('delete','validate','edit')); - $tpl_comment['U_DELETE'] = - add_url_params($url, - array('delete'=>$comment['comment_id']) - ); + $url = + get_root_url() + .'comments.php' + .get_query_string_diff(array('delete','validate','edit', 'pwg_token')); + + $tpl_comment['U_DELETE'] = add_url_params( + $url, + array( + 'delete' => $comment['comment_id'], + 'pwg_token' => get_pwg_token(), + ) + ); } + if (can_manage_comment('edit', $comment['author_id'])) { - $url = get_root_url().'comments.php' - .get_query_string_diff(array('edit', 'delete','validate')); - $tpl_comment['U_EDIT'] = - add_url_params($url, - array('edit'=>$comment['comment_id']) - ); + $url = + get_root_url() + .'comments.php' + .get_query_string_diff(array('edit', 'delete','validate', 'pwg_token')); + + $tpl_comment['U_EDIT'] = add_url_params( + $url, + array( + 'edit' => $comment['comment_id'], + 'pwg_token' => get_pwg_token(), + ) + ); + if (isset($edit_comment) and ($comment['comment_id'] == $edit_comment)) { $tpl_comment['IN_EDIT'] = true; @@ -422,12 +488,18 @@ SELECT id, name, permalink, uppercats } } - if ( is_admin() && $comment['validated'] != 'true') + if (can_manage_comment('validate', $comment['author_id'])) { - $tpl_comment['U_VALIDATE'] = - add_url_params($url, - array('validate'=>$comment['comment_id']) - ); + if ('true' != $comment['validated']) + { + $tpl_comment['U_VALIDATE'] = add_url_params( + $url, + array( + 'validate'=> $comment['comment_id'], + 'pwg_token' => get_pwg_token(), + ) + ); + } } $template->append('comments', $tpl_comment); } diff --git a/feed.php b/feed.php index fcb59c6f0..054fe347e 100644 --- a/feed.php +++ b/feed.php @@ -63,7 +63,7 @@ function ts_to_iso8601($ts) // | initialization | // +-----------------------------------------------------------------------+ -check_input_parameter('feed', @$_GET['feed'], false, '/^[0-9a-z]{50}$/i'); +check_input_parameter('feed', $_GET, false, '/^[0-9a-z]{50}$/i'); $feed_id= isset($_GET['feed']) ? $_GET['feed'] : ''; $image_only=isset($_GET['image_only']); diff --git a/include/constants.php b/include/constants.php index ea5378e94..69b746474 100644 --- a/include/constants.php +++ b/include/constants.php @@ -40,6 +40,9 @@ define('ACCESS_ADMINISTRATOR', 3); define('ACCESS_WEBMASTER', 4); define('ACCESS_CLOSED', 5); +// Sanity checks +define('PATTERN_ID', '/^\d+$/'); + // Table names if (!defined('CATEGORIES_TABLE')) define('CATEGORIES_TABLE', $prefixeTable.'categories'); diff --git a/include/functions.inc.php b/include/functions.inc.php index 91738090f..092fe15a4 100644 --- a/include/functions.inc.php +++ b/include/functions.inc.php @@ -1482,4 +1482,86 @@ function get_icon($date, $is_child_date = false) return $cache['get_icon'][$date] ? $icon : array(); } + +/** + * check token comming from form posted or get params to prevent csrf attacks + * if pwg_token is empty action doesn't require token + * else pwg_token is compare to server token + * + * @return void access denied if token given is not equal to server token + */ +function check_pwg_token() +{ + $valid_token = get_pwg_token(); + $given_token = null; + + if (!empty($_POST['pwg_token'])) + { + $given_token = $_POST['pwg_token']; + } + elseif (!empty($_GET['pwg_token'])) + { + $given_token = $_GET['pwg_token']; + } + if ($given_token != $valid_token) + { + access_denied(); + } +} + +function get_pwg_token() +{ + global $conf; + + return hash_hmac('md5', session_id(), $conf['secret_key']); +} + +/* + * breaks the script execution if the given value doesn't match the given + * pattern. This should happen only during hacking attempts. + * + * @param string param_name + * @param array param_array + * @param boolean is_array + * @param string pattern + * + * @return void + */ +function check_input_parameter($param_name, $param_array, $is_array, $pattern) +{ + $param_value = null; + if (isset($param_array[$param_name])) + { + $param_value = $param_array[$param_name]; + } + + // it's ok if the input parameter is null + if (empty($param_value)) + { + return true; + } + + if ($is_array) + { + if (!is_array($param_value)) + { + fatal_error('[Hacking attempt] the input parameter "'.$param_name.'" should be an array'); + } + + foreach ($param_value as $item_to_check) + { + if (!preg_match($pattern, $item_to_check)) + { + fatal_error('[Hacking attempt] an item is not valid in input parameter "'.$param_name.'"'); + } + } + } + else + { + if (!preg_match($pattern, $param_value)) + { + fatal_error('[Hacking attempt] the input parameter "'.$param_name.'" is not valid'); + } + } +} ?> \ No newline at end of file diff --git a/include/functions_comment.inc.php b/include/functions_comment.inc.php index a35c8ad60..0fadeb1f2 100644 --- a/include/functions_comment.inc.php +++ b/include/functions_comment.inc.php @@ -170,28 +170,25 @@ INSERT INTO '.COMMENTS_TABLE.' $comm['id'] = pwg_db_insert_id(COMMENTS_TABLE); - if (($comment_action=='validate' and $conf['email_admin_on_comment']) or - ($comment_action!='validate' and $conf['email_admin_on_comment_validation'])) + if ($conf['email_admin_on_comment'] + or ($conf['email_admin_on_comment_validation'] and 'moderate' == $comment_action)) { include_once(PHPWG_ROOT_PATH.'include/functions_mail.inc.php'); - $del_url = get_absolute_root_url().'comments.php?delete='.$comm['id']; + $comment_url = get_absolute_root_url().'comments.php?comment_id='.$comm['id']; $keyargs_content = array ( get_l10n_args('Author: %s', stripslashes($comm['author']) ), get_l10n_args('Comment: %s', stripslashes($comm['content']) ), get_l10n_args('', ''), - get_l10n_args('Delete: %s', $del_url) + get_l10n_args('Manage this user comment: %s', $comment_url) ); - if ($comment_action!='validate') + if ('moderate' == $comment_action) { - $keyargs_content[] = - get_l10n_args('', ''); - $keyargs_content[] = - get_l10n_args('Validate: %s', - get_absolute_root_url().'comments.php?validate='.$comm['id']); + $keyargs_content[] = get_l10n_args('', ''); + $keyargs_content[] = get_l10n_args('(!) This comment requires validation', ''); } pwg_mail_notification_admins @@ -212,7 +209,6 @@ INSERT INTO '.COMMENTS_TABLE.' * * @param comment_id */ - function delete_user_comment($comment_id) { $user_where_clause = ''; if (!is_admin()) @@ -337,4 +333,41 @@ function email_admin($action, $comment) $keyargs_content ); } + +function get_comment_author_id($comment_id, $die_on_error=true) +{ + $query = ' +SELECT + author_id + FROM '.COMMENTS_TABLE.' + WHERE id = '.$comment_id.' +;'; + $result = pwg_query($query); + if (pwg_db_num_rows($result) == 0) + { + if ($die_on_error) + { + fatal_error('Unknown comment identifier'); + } + else + { + return false; + } + } + + list($author_id) = pwg_db_fetch_row($result); + + return $author_id; +} + +function validate_user_comment($comment_id) +{ + $query = ' +UPDATE '.COMMENTS_TABLE.' + SET validated = "true" + , validation_date = NOW() + WHERE id = '.$comment_id.' +;'; + pwg_query($query); +} ?> \ No newline at end of file diff --git a/include/functions_user.inc.php b/include/functions_user.inc.php index f3eb0b172..4488294f7 100644 --- a/include/functions_user.inc.php +++ b/include/functions_user.inc.php @@ -1246,19 +1246,44 @@ function is_adviser() } /* - * Return if current user can edit/delete a comment - * @param action edit/delete + * Return if current user can edit/delete/validate a comment + * @param action edit/delete/validate * @return bool */ function can_manage_comment($action, $comment_author_id) { - if (!in_array($action, array('delete','edit'))) { + global $user, $conf; + + if (is_a_guest()) + { + return false; + } + + if (!in_array($action, array('delete','edit', 'validate'))) + { return false; } - return (is_admin() || - (($GLOBALS['user']['id'] == $comment_author_id) - && !is_a_guest() - && $GLOBALS['conf'][sprintf('user_can_%s_comment', $action)])); + + if (is_admin()) + { + return true; + } + + if ('edit' == $action and $conf['user_can_edit_comment']) + { + if ($comment_author_id == $user['id']) { + return true; + } + } + + if ('delete' == $action and $conf['user_can_delete_comment']) + { + if ($comment_author_id == $user['id']) { + return true; + } + } + + return false; } /* diff --git a/include/picture_comment.inc.php b/include/picture_comment.inc.php index 35f686453..439546329 100644 --- a/include/picture_comment.inc.php +++ b/include/picture_comment.inc.php @@ -166,23 +166,25 @@ $validated_clause.' if (can_manage_comment('delete', $row['author_id'])) { - $tpl_comment['U_DELETE'] = - add_url_params($url_self, - array( - 'action'=>'delete_comment', - 'comment_to_delete'=>$row['id'] - ) - ); + $tpl_comment['U_DELETE'] = add_url_params( + $url_self, + array( + 'action'=>'delete_comment', + 'comment_to_delete'=>$row['id'], + 'pwg_token' => get_pwg_token(), + ) + ); } if (can_manage_comment('edit', $row['author_id'])) { - $tpl_comment['U_EDIT'] = - add_url_params($url_self, - array( - 'action'=>'edit_comment', - 'comment_to_edit'=>$row['id'] - ) - ); + $tpl_comment['U_EDIT'] = add_url_params( + $url_self, + array( + 'action'=>'edit_comment', + 'comment_to_edit'=>$row['id'], + 'pwg_token' => get_pwg_token(), + ) + ); if (isset($edit_comment) and ($row['id'] == $edit_comment)) { $tpl_comment['IN_EDIT'] = true; @@ -195,12 +197,14 @@ $validated_clause.' { if ($row['validated'] != 'true') { - $tpl_comment['U_VALIDATE'] = - add_url_params($url_self, - array('action' => 'validate_comment', - 'comment_to_validate' => $row['id'] - ) - ); + $tpl_comment['U_VALIDATE'] = add_url_params( + $url_self, + array( + 'action' => 'validate_comment', + 'comment_to_validate' => $row['id'], + 'pwg_token' => get_pwg_token(), + ) + ); } } $template->append('comments', $tpl_comment); diff --git a/picture.php b/picture.php index 4dd3d4fad..8191dd8ee 100644 --- a/picture.php +++ b/picture.php @@ -311,20 +311,31 @@ UPDATE '.CATEGORIES_TABLE.' } case 'edit_comment' : { + check_pwg_token(); + include_once(PHPWG_ROOT_PATH.'include/functions_comment.inc.php'); - if (isset($_GET['comment_to_edit']) - and is_numeric($_GET['comment_to_edit']) - and (is_admin() || $conf['user_can_edit_comment'])) + + check_input_parameter('comment_to_edit', $_GET, false, PATTERN_ID); + + $author_id = get_comment_author_id($_GET['comment_to_edit']); + + if (can_manage_comment('edit', $author_id)) { if (!empty($_POST['content'])) { - update_user_comment(array('comment_id' => $_GET['comment_to_edit'], - 'image_id' => $page['image_id'], - 'content' => $_POST['content']), - $_POST['key'] - ); + update_user_comment( + array( + 'comment_id' => $_GET['comment_to_edit'], + 'image_id' => $page['image_id'], + 'content' => $_POST['content'] + ), + $_POST['key'] + ); + redirect($url_self); - } else { + } + else + { $edit_comment = $_GET['comment_to_edit']; break; } @@ -332,30 +343,36 @@ UPDATE '.CATEGORIES_TABLE.' } case 'delete_comment' : { + check_pwg_token(); + include_once(PHPWG_ROOT_PATH.'include/functions_comment.inc.php'); - if (isset($_GET['comment_to_delete']) - and is_numeric($_GET['comment_to_delete']) - and (is_admin() || $conf['user_can_delete_comment'])) + + check_input_parameter('comment_to_delete', $_GET, false, PATTERN_ID); + + $author_id = get_comment_author_id($_GET['comment_to_delete']); + + if (can_manage_comment('delete', $author_id)) { delete_user_comment($_GET['comment_to_delete']); } + redirect($url_self); } case 'validate_comment' : { + check_pwg_token(); + include_once(PHPWG_ROOT_PATH.'include/functions_comment.inc.php'); - if (isset($_GET['comment_to_validate']) - and is_numeric($_GET['comment_to_validate']) - and is_admin() and !is_adviser() ) + + check_input_parameter('comment_to_validate', $_GET, false, PATTERN_ID); + + $author_id = get_comment_author_id($_GET['comment_to_delete']); + + if (can_manage_comment('validate', $author_id)) { - $query = ' -UPDATE '.COMMENTS_TABLE.' - SET validated = \'true\' - , validation_date = NOW() - WHERE id='.$_GET['comment_to_validate'].' -;'; - pwg_query( $query ); + validate_user_comment($_GET['comment_to_validate']); } + redirect($url_self); } diff --git a/search.php b/search.php index 97c5b1fa2..1cb9726e0 100644 --- a/search.php +++ b/search.php @@ -71,7 +71,7 @@ if (isset($_POST['submit'])) if (isset($_POST['tags'])) { - check_input_parameter('tags', $_POST['tags'], true, PATTERN_ID); + check_input_parameter('tags', $_POST, true, PATTERN_ID); $search['fields']['tags'] = array( 'words' => $_POST['tags'], @@ -92,7 +92,7 @@ if (isset($_POST['submit'])) if (isset($_POST['cat'])) { - check_input_parameter('cat', $_POST['cat'], true, PATTERN_ID); + check_input_parameter('cat', $_POST, true, PATTERN_ID); $search['fields']['cat'] = array( 'words' => $_POST['cat'], -- cgit v1.2.3