From 70de6817109547c73088fa7f042752da6331a5ea Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Tue, 31 Oct 2017 11:07:59 -0400 Subject: [PATCH 01/21] WIP: Enforce wp_slash() on functions that expect slashed data --- WordPress/Sniffs/WP/ExpectedSlashedSniff.php | 774 ++++++++++++++++++ .../Tests/WP/ExpectedSlashedUnitTest.inc | 93 +++ .../Tests/WP/ExpectedSlashedUnitTest.php | 60 ++ 3 files changed, 927 insertions(+) create mode 100644 WordPress/Sniffs/WP/ExpectedSlashedSniff.php create mode 100644 WordPress/Tests/WP/ExpectedSlashedUnitTest.inc create mode 100644 WordPress/Tests/WP/ExpectedSlashedUnitTest.php diff --git a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php new file mode 100644 index 0000000000..3361e466b6 --- /dev/null +++ b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php @@ -0,0 +1,774 @@ + true, + T_DOUBLE_CAST => true, + T_BOOL_CAST => true, + ); + + /** + * Tokens that are implicitly slashed and so don't need to be flagged. + * + * @since + * + * @var int[] + */ + protected $slashed_tokens = array( + T_CONSTANT_ENCAPSED_STRING => true, // TODO + T_LNUMBER => true, + T_MINUS => true, + T_TRUE => true, + T_FALSE => true, + T_NULL => true, + ); + + /** + * The list functions and their args that are expected to be slashed. + * + * @since + * + * @var array[] + */ + public static $expectedSlashedFunctionArgs = array( + // Uses add_metadata(). + 'add_comment_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + // Uses wp_unslash(). + 'add_metadata' => array( 3 => 'meta_key', 4 => 'meta_value' ), + // Uses wp_unslash(). + 'add_ping' => array( 2 => 'uri' ), + // Uses add_metadata(). + 'add_post_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + // Uses add_metadata(). + 'add_term_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + // Uses add_metadata(). + 'add_user_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + // These are directly interpolated into a database query. Failure to slash + // will result in SQL injection!! + 'check_comment' => array( 1 => 'author', 2 => 'email' ), + // Uses stripslashes(). + 'comment_exists' => array( 1 => 'comment_author' ), + // Uses delete_metadata(). + 'delete_comment_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + // Uses wp_unslash(). + 'delete_metadata' => array( 3 => 'meta_key', 4 => 'meta_value' ), + // Uses delete_metadata(). + 'delete_post_meta_by_key' => array( 1 => 'post_meta_key' ), + // Uses delete_metadata(). + 'delete_post_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + // Uses delete_metadata(). + 'delete_term_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + // Uses delete_metadata(). + 'delete_user_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + // Uses delete_user_meta(). + 'delete_user_option' => array( 2 => 'option_name' ), + // Expects POST data. + 'edit_post' => array( 1 => 'post_data' ), + // Uses get_term_by( 'name' ). + 'get_cat_ID' => array( 1 => 'category_name' ), + // Uses get_search_feed_link(). + 'get_search_comments_feed_link' => array( 1 => array( 'search_query' ) ), + // Uses get_search_link(). + 'get_search_feed_link' => array( 1 => 'search_query' ), + // Uses stripslashes(). + 'get_search_link' => array( 1 => 'query' ), + // Uses wp_unslash() when $field is 'name'. + 'get_term_by' => array( 2 => 'value' ), + // Uses wp_unslash(). + 'install_blog' => array( 2 => 'blog_title' ), + // Uses wp_get_nav_menu_object(). + 'is_nav_menu' => array( 1 => 'menu' ), + // Uses wp_unslash(). + 'post_exists' => array( 1 => 'title', 2 => 'content', 3 => 'date' ), + // Uses update_post_meta() when the $file isn't empty. + 'update_attached_file' => array( 2 => 'file' ), + // Uses update_metadata(). + 'update_comment_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + // Uses wp_unslash(). + 'update_metadata' => array( 3 => 'meta_key', 4 => 'meta_value' ), + // Uses wp_unslash(). + 'update_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + // Uses update_metadata(). + 'update_post_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + // Uses update_metadata(). + 'update_term_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + // Uses update_metadata(). + 'update_user_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + // Uses wp_unslash() when a string is passed; also accepts term ID. + 'term_exists' => array( 1 => 'term' ), + // Uses update_user_meta(). + 'update_user_option' => array( 2 => 'option_name', 3 => 'newvalue' ), + // Uses wp_set_object_terms(). + 'wp_add_object_terms' => array( 2 => 'terms' ), + // Uses wp_update_nav_menu_object(). + 'wp_create_nav_menu' => array( 1 => 'menu_name' ), + // Uses wp_unslash(). + 'wp_create_post_autosave' => array( 1 => 'post_data' ), + // Uses wp_get_nav_menu_object(). + 'wp_delete_nav_menu' => array( 1 => 'menu' ), + // Uses wp_get_nav_menu_object(). + 'wp_get_nav_menu_items' => array( 1 => 'menu' ), + // Uses get_term_by( 'name' ) if $menu is not a term ID or slug. + 'wp_get_nav_menu_object' => array( 1 => 'menu' ), + // Uses wp_insert_post(). + 'wp_insert_attachment' => array( 1 => 'args' ), + // Uses wp_unslash(). + 'wp_insert_comment' => array( 1 => 'commentdata' ), + // Uses wp_unslash(). + 'wp_insert_link' => array( 1 => 'linkdata' ), + // Uses wp_unslash(). + 'wp_insert_post' => array( 1 => 'postarr' ), + // Uses wp_unslash(). + 'wp_insert_term' => array( 1 => 'term' ), + // Uses wp_unslash(), update_user_meta(). + 'wp_insert_user' => array( 1 => 'userdata' ), + // Uses wp_insert_comment() and wp_allow_comment(). + 'wp_new_comment' => array( 1 => 'commentdata' ), + // Uses term_exists(). The docs for wp_remove_object_terms() says that it + // takes only term slugs or IDs, but it is also possible to pass in the term + // names, and in that case they must be slashed. + 'wp_remove_object_terms' => array( 2 => 'terms' ), + // Uses term_exists(), and wp_insert_term() if the term doesn't exist and is + // a string. The docs for wp_set_object_terms() says that it takes only term + // slugs or IDs, but it is also possible to pass in the term names, and in + // that case they must be slashed. + 'wp_set_object_terms' => array( 2 => 'terms' ), + // Uses wp_set_object_terms(). + 'wp_set_post_terms' => array( 2 => 'terms' ), + // Uses update_post_meta(). + 'wp_update_attachment_metadata' => array( 2 => 'data' ), + // Uses wp_unslash(). + 'wp_update_comment' => array( 1 => 'commentarr' ), + // Uses wp_insert_post(). If the $postarr is actually a post object and not + // an array, then it should be unslashed instead. + 'wp_update_post' => array( 1 => 'postarr' ), + // Uses wp_insert_user(). + 'wp_update_user' => array( 1 => 'userdata' ), + // Uses install_blog(). + 'wpmu_create_blog' => array( 3 => 'title' ), + // Uses wp_unslash(). + 'wpmu_validate_blog_signup' => array( 2 => 'blog_title' ), + // Uses wp_unslash(). + 'wpmu_welcome_notification' => array( 4 => 'title' ), + // Uses wp_unslash(). + 'WP_Press_This::side_load_images' => array( 2 => 'content' ), + // Uses wp_unslash(). + 'WP_Customize_Setting::sanitize' => array( 1 => 'value' ), + ); + + public $filters = array( + // Result passed through wp_unslash(). + 'pre_comment_author_name' => array( 1 => 'author_cookie' ), + // Result passed through wp_unslash(). + 'pre_comment_author_email' => array( 1 => 'author_email_cookie' ), + // Result passed through wp_unslash(). + 'pre_comment_author_url' => array( 1 => 'author_url_cookie' ), + // Result passed through wp_unslash(). + 'add_ping' => array( 1 => 'new' ), + ); + + /** + * A list of functions and their args that are expected to be partly slashed. + * + * Sometimes a function takes an argument that is an array, and some of the + * values in the array are expected to be slashed while others do not need to be + * (because they are a type of value that doesn't need to be slashed, like an + * integer or boolean, perhaps). So it is OK to pass the entire array through + * a slashing function, but it's also permissible to slash only those args that + * need to be (i.e., when the array is being passed to the function directly, + * rather than being assigned to a variable first; then we can check each key + * value pair and only flag those that really need to be slashed). + * + * This is a list of those functions. For each function, there is an array which + * is indexed by the number of the argument which is expected to be an array with + * mixed slashing. The values are lists of the keys within that array that are + * expected to be slashed (any other keys are expected unslashed). + * + * @since + * + * @var array[] + */ + public static $partlySlashedFunctionArgs = array( + // Uses query_posts(), 'q' is passed as 's'. + '_wp_ajax_menu_quick_search' => array( 1 => array( 'q' ) ), + // Uses get_posts() with the $args if they are an array. + 'get_children' => array( 1 => array( 's', 'title' ) ), + // Uses get_term_by( 'name' ). All of the other args are either integers or + // accept slug-like values. + 'get_bookmarks' => array( 1 => array( 'category_name' ) ), + // Uses wp_unslash() on these. All of the other args are either integers or + // accept slug-like values. + 'get_pages' => array( 1 => array( 'meta_key', 'meta_value' ) ), + // Uses WP_Query::query(). + 'get_posts' => array( 1 => array( 's', 'title' ) ), + // Uses WP_Query::query(). + 'query_posts' => array( 1 => array( 's', 'title' ) ), + // Uses wp_unslash() on some of these. All of the other args are either + // integers, slugs, or dates. + 'wp_allow_comment' => array( + 1 => array( 'comment_author', 'comment_author_email', 'comment_author_url', 'comment_author_IP', 'comment_content', 'comment_agent' ), + ), + // Uses get_posts(). + 'wp_get_nav_menu_items' => array( 2 => array( 's', 'title' ) ), + // Uses get_children(). + 'wp_get_post_revisions' => array( 2 => array( 's', 'title' ) ), + // Uses get_posts(). + 'wp_get_recent_posts' => array( 1 => array( 's', 'title' ) ), + // Uses wp_unslash() on this. All of the other args are integers or slugs. + // The 'name' arg is also expected slashed, but this is always overridden by + // $term. + 'wp_insert_term' => array( 3 => array( 'description' ) ), + // Uses wp_insert_post() or wp_update_post(). All other values are slugs or + // integers. + 'wp_update_nav_menu_item' => array( + 3 => array( 'menu-item-description', 'menu-item-attr-title', 'menu-item-title' ) + ), + // Uses get_term_by( 'name' ) with 'menu-name' and also passes the data to + // wp_insert_term() if the menu doesn't exist, or else wp_update_term(). + 'wp_update_nav_menu_object' => array( 2 => array( 'description', 'menu-name' ) ), + // Uses wp_unslash() on these. All of the other args are integers or slugs. + 'wp_update_term' => array( 3 => array( 'description', 'name' ) ), + // Uses WP_Query::__construct(). + 'WP_Customize_Nav_Menus::search_available_items_query' => array( 1 => array( 's' ) ), + // Uses WP_Query::query(). + 'WP_Query::__construct' => array( 1 => array( 's', 'title' ) ), + // WP_Query::get_posts() uses stripslashes() on the 'title', and + // WP_Query::parse_search() uses stripslashes() on 's'. All other args are + // either integers, booleans, or slug-like strings. + 'WP_Query::parse_query' => array( 1 => array( 's', 'title' ) ), + // WP_Query::get_posts() uses stripslashes() on the 'title', and + // WP_Query::parse_search() uses stripslashes() on 's'. All other args are + // either integers, booleans, or slug-like strings. + 'WP_Query::query' => array( 1 => array( 's', 'title' ) ), + // Uses WP_Query::query(). + '_WP_Editors::wp_link_query' => array( 1 => array( 's' ) ), + ); + + /** + * A list of functions and their args that are expected to be partly slashed and + * partly unslashed. + * + * Sometimes a function takes an argument that is an array, and some of the + * values in the array are expected to be slashed while others specifically + * expected to be unslashed. + * + * See self::$partlySlashedFunctionArgs for an explanation of the array format. + * + * @since + * + * @var array[] + */ + public static $mixedSlashedFunctionArgs = array( + // Uses get_bookmarks(). When 'categorize' is true, passing 'categroy_name' + // doesn't make sense, so this only applies when 'categorize' false. In fact, + // when 'categorize' is true, the 'category_name' is passed to get_terms() as + // 'name__like', which is expected unslashed. So it is only expected slashed + // when 'categorize' is false. + 'wp_list_bookmarks' => array( + 1 => array( + 'slashed' => array( 'category_name' ), + 'unslashed' => array( + 'title_li', + 'title_before', + 'title_after', + 'class', + 'category_before', + 'category_after', + ), + ), + ), + // Uses get_pages(). + 'wp_dropdown_pages' => array( + 1 => array( + 'slashed' => array( 'meta_key', 'meta_value' ), + 'unslashed' => array( + 'selected', + 'name', + 'id', + 'show_option_none', + 'show_option_no_change', + 'option_none_value', + ), + ), + ), + // Uses get_pages(). + 'wp_list_pages' => array( + 1 => array( + 'slashed' => array( 'meta_key', 'meta_value' ), + 'unslashed' => array( 'date_format', 'link_after', 'link_before', 'title_li' ), + ), + ), +// 'wp_insert_category' => array( 3 => array( 'name', 'description' ) ), + ); + + /** + * Functions that slash data. + * + * @since + * + * @var array + */ + public static $slashingFunctions = array( + 'wp_slash' => true, + ); + + /** + * Functions that return slashed data automatically. + * + * @since + * + * @var array + */ + public static $autoSlashingFunctions = array( + 'esc_url_raw' => true, + 'esc_url' => true, + 'get_current_user_id' => true, + 'sanitize_key' => true, + 'sanitize_title' => true, + 'sanitize_title_with_dashes' => true, + 'time' => true, + ); + + /** + * Groups of functions to restrict. + * + * @since + * + * @return array + */ + public function getGroups() { + $this->target_functions = array_merge( + self::$expectedSlashedFunctionArgs + , self::$partlySlashedFunctionArgs + , self::$mixedSlashedFunctionArgs + ); + + return parent::getGroups(); + } + + /** + * Processes this test, when one of its tokens is encountered. + * + * @since + * + * @param int $stackPtr The position of the current token in the stack. + * + * @return int|void Integer stack pointer to skip forward or void to continue + * normal file processing. + */ + public function process_token( $stackPtr ) { + + if ( $this->has_whitelist_comment( 'slashing', $stackPtr ) ) { + return; + } + + parent::process_token( $stackPtr ); + + } // End process_token(). + + /** + * Process the parameters of a matched function. + * + * @since + * + * @param int $stackPtr The position of the current token in the stack. + * @param array $group_name The name of the group which was matched. + * @param string $matched_content The token content (function name) which was matched. + * @param array $parameters Array with information about the parameters. + * + * @return void + */ + public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { + + // Check if we are inside a function. + $function_ptr = $this->phpcsFile->getCondition( $stackPtr, T_FUNCTION ); + + if ( $function_ptr ) { + + $function_declaration_name = $this->phpcsFile->findNext( + Tokens::$emptyTokens, + $function_ptr + 1, + null, + true, + null, + true + ); + + $function_declaration_name = $this->tokens[ $function_declaration_name ]['content']; + + // If we are inside a function that expects slashed arguments we don't + // really need to flag any internal function calls, because if any of + // them expects slashed data but isn't receiving it, that is likely the + // reason that the wrapping function is in this list. + if ( + isset( self::$expectedSlashedFunctionArgs[ $function_declaration_name ] ) +// || isset( self::$partlySlashedFunctionArgs[ $function_declaration_name ] ) +// || isset( self::$mixedSlashedFunctionArgs[ $function_declaration_name ] ) + ) { + return; + } + } + + $function_name = $matched_content; + + if ( + isset( self::$mixedSlashedFunctionArgs[ $function_name ] ) + || isset( self::$partlySlashedFunctionArgs[ $function_name ] ) + ) { + $this->process_mixed_function_args( $parameters, $function_name ); + } + + if ( isset( self::$expectedSlashedFunctionArgs[ $function_name ] ) ) { + $this->process_expected_slashed_args( $parameters, $function_name ); + } + + } // End process_parameters(). + + /** + * Process a function that expects some arguments with mixed slashing. + * + * @since + * + * @param array $parameters Info on the function parameters. + * @param string $function_name The function name. + */ + protected function process_mixed_function_args( $parameters, $function_name ) { + + $phpcsFile = $this->phpcsFile; + $tokens = $this->tokens; + + if ( isset( self::$mixedSlashedFunctionArgs[ $function_name ] ) ) { + $args = self::$mixedSlashedFunctionArgs[ $function_name ]; + $is_mixed = true; + } else { + $args = self::$partlySlashedFunctionArgs[ $function_name ]; + $is_mixed = false; + } + + foreach ( $args as $arg_index => $keys ) { + + if ( ! isset( $parameters[ $arg_index ]['start'] ) ) { + break; + } + + $argPtr = $parameters[ $arg_index ]['start']; + + if ( ! $argPtr ) { + break; + } + + $arg_start = $phpcsFile->findNext( + Tokens::$emptyTokens, + $argPtr + 1, + null, + true, + null, + true + ); + + if ( $is_mixed ) { + $slashed_keys = $keys['slashed']; + $unslashed_keys = $keys['unslashed']; + } else { + $slashed_keys = $keys; + $unslashed_keys = array(); + } + + // If this is the array declaration itself, we can check per-key. + // Otherwise, we can only give a general error/warning. + if ( T_ARRAY !== $tokens[ $arg_start ]['code'] ) { + + if ( $is_mixed ) { + + $phpcsFile->addWarning( + '%s() expects the value of %s to be slashed with wp_slash(), and %s to be unslashed.', + $argPtr, + 'ExpectedMixed', + array( + $function_name, + implode( ', ', $slashed_keys ), + implode( ', ', $unslashed_keys ) + ) + ); + + } else { + + if ( + ! isset( self::$slashingFunctions[ $tokens[ $arg_start ]['content'] ] ) + && ! isset( self::$autoSlashingFunctions[ $tokens[ $arg_start ]['content'] ] ) + ) { + + $phpcsFile->addError( + '%s() expects the value of %s to be slashed with wp_slash().', + $argPtr, + 'ExpectedPartlySlashed', + array( $function_name, implode( ', ', $slashed_keys ) ) + ); + } + } + + break; + } + + $array_opener = $phpcsFile->findNext( + Tokens::$emptyTokens, + $arg_start + 1, + null, + true, + null, + true + ); + + // This likely indicates a syntax error. + if ( ! isset( $tokens[ $array_opener ]['parenthesis_closer'] ) ) { + + $phpcsFile->addError( + 'Mising closing parenthesis for array.', + $argPtr, + 'MissingArrayClosingParen' + ); + + break; + } + + $start = $array_opener; + + while ( + $next_double_arrow = $phpcsFile->findNext( + T_DOUBLE_ARROW, + $start + 1, + $tokens[ $array_opener ]['parenthesis_closer'] + ) + ) { + $is_slashed = false; + $start = $next_double_arrow + 1; + + $value_ptr = $phpcsFile->findNext( + Tokens::$emptyTokens, + $next_double_arrow + 1, + null, + true, + null, + true + ); + + // These tokens are implicitly slashed, so we don't need to slash them. + if ( + isset( $this->slashed_tokens[ $tokens[ $value_ptr ]['code'] ] ) + || isset( $this->slashed_casts[ $tokens[ $value_ptr ]['code'] ] ) + ) { + continue; + } + + $key_ptr = $phpcsFile->findPrevious( + Tokens::$emptyTokens, + $next_double_arrow - 1, + null, + true, + null, + true + ); + + if ( T_CONSTANT_ENCAPSED_STRING !== $tokens[ $key_ptr ]['code'] ) { + continue; + } + + $key_name = trim( $tokens[ $key_ptr ]['content'], '\'"' ); + + if ( + isset( self::$slashingFunctions[ $tokens[ $value_ptr ]['content'] ] ) + || isset( self::$autoSlashingFunctions[ $tokens[ $value_ptr ]['content'] ] ) + ) { + $is_slashed = true; + } + + if ( ! $is_slashed && in_array( $key_name, $slashed_keys, true ) ) { + + $phpcsFile->addError( + '%s() expects the value of %s to be slashed with wp_slash().', + $value_ptr, + 'ExpectedKeySlashed', + array( $function_name, $key_name ) + ); + + } elseif ( $is_slashed && in_array( $key_name, $unslashed_keys, true ) ) { + + $phpcsFile->addError( + '%s() expects the value of %s to be unslashed.', + $value_ptr, + 'ExpectedKeyUnslashed', + array( $function_name, $key_name ) + ); + } + } + } + + } // End process_mixed_function_args() + + /** + * Process a function that expects some args to be slashed. + * + * @since + * + * @param array $parameters Info on the function parameters. + * @param string $function_name The function name. + */ + protected function process_expected_slashed_args( $parameters, $function_name ) { + + $phpcsFile = $this->phpcsFile; + $tokens = $this->tokens; + + // Special handling for get_term_by( 'name' ). + if ( 'get_term_by' === $function_name && isset( $parameters[1] ) ) { + + $byPtr = $phpcsFile->findNext( + Tokens::$emptyTokens, + $parameters[1]['start'] + 1, + null, + true, + null, + true + ); + + // If we know what we are getting the term by, and it isn't 'name', then + // we can skip this. Otherwise we're getting it by name or the arg is + // supplied dynamically, so we don't know whether we are or not. + if ( + T_CONSTANT_ENCAPSED_STRING === $tokens[ $byPtr ]['code'] + && trim( $tokens[ $byPtr ]['content'], '\'"' ) !== 'name' + ) { + return; + } + } + + foreach ( self::$expectedSlashedFunctionArgs[ $function_name ] as $arg_index => $name ) { + + if ( ! isset( $parameters[ $arg_index ] ) ) { + break; + } + + $argPtr = $parameters[ $arg_index ]['start']; + + $in_cast = false; + $watch = true; + + for ( $i = $argPtr; $i <= $parameters[ $arg_index ]['end']; $i++ ) { + + if ( T_COMMA === $tokens[ $i ]['code'] ) { + $watch = true; + continue; + } + + // If we're not watching right now, do nothing. + if ( ! $watch ) { + + // Wake up on concatenation characters, another part to check. + if ( T_STRING_CONCAT === $tokens[ $i ]['code'] ) { + $watch = true; + } + + continue; + } + + // Ignore whitespaces and comments. + if ( in_array( $tokens[ $i ]['code'], Tokens::$emptyTokens ) ) { + continue; + } + + // Skip to the end of a function call if it has been casted to a safe value. + if ( $in_cast && T_OPEN_PARENTHESIS === $tokens[ $i ]['code'] ) { + $i = $tokens[ $i ]['parenthesis_closer']; + $in_cast = false; + continue; + } + + // Handle arrays for those functions that accept them. + if ( T_ARRAY === $tokens[ $i ]['code'] ) { + $i ++; // Skip the opening parenthesis. + continue; + } + + if ( in_array( $tokens[ $i ]['code'], array( T_DOUBLE_ARROW, T_CLOSE_PARENTHESIS, T_STRING_CONCAT ) ) ) { + continue; + } + + // Allow tokens that are implicitly slashed. + if ( isset( $this->slashed_tokens[ $tokens[ $i ]['code'] ] ) ) { + continue; + } + + // If we were watching before, stop now. That way we'll error once + // for a set of unslashed tokens, instead of for each of the tokens. + $watch = false; + + // Allow int/float/bool casted variables. + if ( isset( $this->slashed_casts[ $tokens[ $i ]['code'] ] ) ) { + $in_cast = true; + continue; + } + + $content = $tokens[ $i ]['content']; + + // If this is a function call. + if ( T_STRING === $tokens[ $i ]['code'] ) { + + // We can fast-forward to the end of this function call, we don't + // need to check each token because we're going to check whether + // the result of the function is slashed (below). + $paren_opener = $phpcsFile->findNext( Tokens::$emptyTokens, $i + 1, null, true ); + + if ( isset( $tokens[ $paren_opener ]['parenthesis_closer'] ) ) { + $i = $tokens[ $paren_opener ]['parenthesis_closer']; + } + + // If the function is a slashing function we continue to the next + // token instead of giving an error. + if ( + isset( self::$slashingFunctions[ $content ] ) + || isset( self::$autoSlashingFunctions[ $content ] ) + ) { + continue; + } + } + + $phpcsFile->addError( + '%s() expects the value of the $%s arg to be slashed with wp_slash(); %s found.', + $i, + 'MissingSlashing', + array( $function_name, $name, $content ) + ); + } + } + + } // End process_expected_slashed_args() + +} // End class. diff --git a/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc b/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc new file mode 100644 index 0000000000..1d953de0a9 --- /dev/null +++ b/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc @@ -0,0 +1,93 @@ + 'value', 'another' => 'val' ) ); // OK +add_post_meta( $post_id, 'key', array( 1 => 2, 3 => -4 ) ); // OK + +add_post_meta( $post_id, 'key', array( $value, $test ) ); // Bad +add_post_meta( $post_id, 'key', array( wp_slash( $value ), $test ) ); // Bad +add_post_meta( $post_id, 'key', array( wp_slash( $value ), wp_slash( $test ) ) ); // OK +add_post_meta( $post_id, 'key', wp_slash( array( $value, $test ) ) ); // OK + +wp_insert_comment( $data ); // Bad +wp_insert_comment( wp_slash( $data ) ); // OK + +// Partly slashed function. +wp_update_term( $term_id, 'tag' ); // OK +wp_update_term( $term_id, 'tag', $data ); // Bad +wp_update_term( $term_id, 'tag', wp_slash( $data ) ); // OK +wp_update_term( $term_id, 'tag', array( 'parent' => $parent ) ); // OK +wp_update_term( $term_id, 'tag', array( 'parent' => $parent, 'name' => $name ) ); // Bad +wp_update_term( $term_id, 'tag', array( 'name' => $name ) ); // Bad +wp_update_term( $term_id, 'tag', array( 'parent' => $parent, 'name' => wp_slash( $name ) ) ); // OK +wp_update_term( $term_id, 'tag', array( 'name' => wp_slash( $name ) ) ); // OK +wp_update_term( $term_id, 'tag', array( 'parent' => wp_slash( $parent ) ) ); // OK +wp_update_term( $term_id, 'tag', array( 'parent' => wp_slash( $parent ), 'name' => wp_slash( $name ) ) ); // OK +wp_update_term( $term_id, 'tag', array( 'name' => 'Name' ) ); // OK +wp_update_term( $term_id, 'tag', array( 'name' => -58 ) ); // OK + +// Mixed slashing function. +wp_list_pages(); // OK. +wp_list_pages( $data ); // Warning. +wp_list_pages( wp_slash( $data ) ); // Warning. +wp_list_pages( array( 'meta_value' => $value, 'link_before' => $before ) ); // Bad. +wp_list_pages( array( 'meta_value' => wp_slash( $value ), 'link_before' => $before ) ); // OK. +wp_list_pages( wp_slash( array( 'meta_value' => $value, 'link_before' => $before ) ) ); // Warning. +wp_list_pages( array( 'meta_value' => wp_slash( $value ), 'link_before' => wp_slash( $before ) ) ); // Bad. +wp_list_pages( array( 'meta_value' => $value, 'link_before' => wp_slash( $before ) ) ); // Bad x 2. + +// Function declarations and function calls within the declaration of an ignored +// function should both be ignored. +function add_comment_meta( $comment_id, $meta_key, $meta_value, $unique = false ) { // OK + return add_metadata( 'comment', $comment_id, $meta_key, $meta_value, $unique ); // OK +} + +function get_bookmarks( $r ) { // OK + return get_term_by( 'name', $r['category_name'], 'link_category' ); // OK +} + +delete_metadata( 'user', 0, 'session_tokens', false, true ); // OK +delete_metadata( 'user', 0, 'session_tokens', array( false ), true ); // OK + +// We need to make sure we don't blow up when an arg isn't set (meta value is optional). +delete_post_meta( $post_ID, 'meta_key' ); // OK +get_pages(); // OK + +// Whitelisting. +add_post_meta( $post_id, 'key', $value ); // WPCS: slashing OK + +// Some functions are ignored. +add_post_meta( $post_id, 'key', time() ); // OK +add_post_meta( $post_id, 'key', 'a-' . sanitize_key( 'lkjlkj' ) ); // OK + +// Method names matching flagged functions should be ignored. +$obj->get_children( $args ); // OK + +// Only an error when this is 'name'. +get_term_by( 'slug', $term ); // OK. +get_term_by( 'name', $term ); // Bad. +get_term_by( $by, $term ); // Bad. diff --git a/WordPress/Tests/WP/ExpectedSlashedUnitTest.php b/WordPress/Tests/WP/ExpectedSlashedUnitTest.php new file mode 100644 index 0000000000..b34b9edb01 --- /dev/null +++ b/WordPress/Tests/WP/ExpectedSlashedUnitTest.php @@ -0,0 +1,60 @@ + 1, + 9 => 1, + 16 => 1, + 17 => 1, + 19 => 1, + 22 => 1, + 31 => 2, + 32 => 1, + 36 => 1, + 41 => 1, + 44 => 1, + 45 => 1, + 57 => 1, + 60 => 1, + 61 => 2, + 92 => 1, + 93 => 1, + ); + } + + /** + * @since + */ + public function getWarningList() { + return array( + 55 => 1, + 56 => 1, + 59 => 1, + 70 => 1, + ); + } + +} // End class. From 832fc4842973d8e30622abc56294b3051f586f0b Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Tue, 31 Oct 2017 11:51:22 -0400 Subject: [PATCH 02/21] Detect superglobals and don't require slashing --- WordPress/Sniffs/WP/ExpectedSlashedSniff.php | 6 ++++++ WordPress/Tests/WP/ExpectedSlashedUnitTest.inc | 3 +++ 2 files changed, 9 insertions(+) diff --git a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php index 3361e466b6..be4d1e71d1 100644 --- a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php +++ b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php @@ -185,6 +185,7 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'WP_Customize_Setting::sanitize' => array( 1 => 'value' ), ); + // TODO public $filters = array( // Result passed through wp_unslash(). 'pre_comment_author_name' => array( 1 => 'author_cookie' ), @@ -758,6 +759,11 @@ protected function process_expected_slashed_args( $parameters, $function_name ) ) { continue; } + + } elseif ( T_VARIABLE === $tokens[ $i ]['code'] ) { + if ( in_array( $content, $this->input_superglobals, true ) ) { + continue; + } } $phpcsFile->addError( diff --git a/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc b/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc index 1d953de0a9..f71f3b2352 100644 --- a/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc +++ b/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc @@ -91,3 +91,6 @@ $obj->get_children( $args ); // OK get_term_by( 'slug', $term ); // OK. get_term_by( 'name', $term ); // Bad. get_term_by( $by, $term ); // Bad. + +// Slashed superglobals. +wpmu_validate_blog_signup( $_POST['blogname'], $_POST['blog_title'], $user ); // OK. From 728419a6f636c5b0e4e2ddffb8cf7c928cc937e8 Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Tue, 31 Oct 2017 12:01:30 -0400 Subject: [PATCH 03/21] Don't expect whitespace --- WordPress/Sniffs/WP/ExpectedSlashedSniff.php | 6 +----- WordPress/Tests/WP/ExpectedSlashedUnitTest.inc | 3 +++ 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php index be4d1e71d1..301833b2e5 100644 --- a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php +++ b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php @@ -483,13 +483,9 @@ protected function process_mixed_function_args( $parameters, $function_name ) { $argPtr = $parameters[ $arg_index ]['start']; - if ( ! $argPtr ) { - break; - } - $arg_start = $phpcsFile->findNext( Tokens::$emptyTokens, - $argPtr + 1, + $argPtr, null, true, null, diff --git a/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc b/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc index f71f3b2352..2012f98e05 100644 --- a/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc +++ b/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc @@ -94,3 +94,6 @@ get_term_by( $by, $term ); // Bad. // Slashed superglobals. wpmu_validate_blog_signup( $_POST['blogname'], $_POST['blog_title'], $user ); // OK. + +// Without whitespace. +get_bookmarks(array("category" => $cat->term_id)); // OK. From dc7310fbac91f50beabe54f7cd9185f56444c865 Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Tue, 31 Oct 2017 15:15:51 -0400 Subject: [PATCH 04/21] Add a couple of filters that expect slashed args --- WordPress/Sniffs/WP/ExpectedSlashedSniff.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php index 301833b2e5..9af41f63c5 100644 --- a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php +++ b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php @@ -195,6 +195,10 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'pre_comment_author_url' => array( 1 => 'author_url_cookie' ), // Result passed through wp_unslash(). 'add_ping' => array( 1 => 'new' ), + // Result passed to wp_list_pages(). + 'widget_pages_args' => array( 1 => 'args' ), + // Result passed to wp_list_bookmarks(). + 'widget_links_args' => array( 1 => 'widget_links_args' ), ); /** From 290f96a158ec620ea6a85e95ccbdb0c239f52801 Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Tue, 31 Oct 2017 15:35:57 -0400 Subject: [PATCH 05/21] Give warnings instead of errors when inside a slashed function --- WordPress/Sniffs/WP/ExpectedSlashedSniff.php | 107 +++++++++++------- .../Tests/WP/ExpectedSlashedUnitTest.inc | 4 +- .../Tests/WP/ExpectedSlashedUnitTest.php | 1 + 3 files changed, 70 insertions(+), 42 deletions(-) diff --git a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php index 9af41f63c5..019b5ad212 100644 --- a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php +++ b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php @@ -414,35 +414,6 @@ public function process_token( $stackPtr ) { */ public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { - // Check if we are inside a function. - $function_ptr = $this->phpcsFile->getCondition( $stackPtr, T_FUNCTION ); - - if ( $function_ptr ) { - - $function_declaration_name = $this->phpcsFile->findNext( - Tokens::$emptyTokens, - $function_ptr + 1, - null, - true, - null, - true - ); - - $function_declaration_name = $this->tokens[ $function_declaration_name ]['content']; - - // If we are inside a function that expects slashed arguments we don't - // really need to flag any internal function calls, because if any of - // them expects slashed data but isn't receiving it, that is likely the - // reason that the wrapping function is in this list. - if ( - isset( self::$expectedSlashedFunctionArgs[ $function_declaration_name ] ) -// || isset( self::$partlySlashedFunctionArgs[ $function_declaration_name ] ) -// || isset( self::$mixedSlashedFunctionArgs[ $function_declaration_name ] ) - ) { - return; - } - } - $function_name = $matched_content; if ( @@ -528,7 +499,7 @@ protected function process_mixed_function_args( $parameters, $function_name ) { && ! isset( self::$autoSlashingFunctions[ $tokens[ $arg_start ]['content'] ] ) ) { - $phpcsFile->addError( + $this->addError( '%s() expects the value of %s to be slashed with wp_slash().', $argPtr, 'ExpectedPartlySlashed', @@ -551,13 +522,6 @@ protected function process_mixed_function_args( $parameters, $function_name ) { // This likely indicates a syntax error. if ( ! isset( $tokens[ $array_opener ]['parenthesis_closer'] ) ) { - - $phpcsFile->addError( - 'Mising closing parenthesis for array.', - $argPtr, - 'MissingArrayClosingParen' - ); - break; } @@ -614,7 +578,7 @@ protected function process_mixed_function_args( $parameters, $function_name ) { if ( ! $is_slashed && in_array( $key_name, $slashed_keys, true ) ) { - $phpcsFile->addError( + $this->addError( '%s() expects the value of %s to be slashed with wp_slash().', $value_ptr, 'ExpectedKeySlashed', @@ -623,7 +587,7 @@ protected function process_mixed_function_args( $parameters, $function_name ) { } elseif ( $is_slashed && in_array( $key_name, $unslashed_keys, true ) ) { - $phpcsFile->addError( + $this->addError( '%s() expects the value of %s to be unslashed.', $value_ptr, 'ExpectedKeyUnslashed', @@ -766,7 +730,7 @@ protected function process_expected_slashed_args( $parameters, $function_name ) } } - $phpcsFile->addError( + $this->addError( '%s() expects the value of the $%s arg to be slashed with wp_slash(); %s found.', $i, 'MissingSlashing', @@ -777,4 +741,67 @@ protected function process_expected_slashed_args( $parameters, $function_name ) } // End process_expected_slashed_args() + /** + * Records an error against a specific token in the file being sniffed. + * + * @since ${PROJECT_VERSION} + * + * @param string $error The error message. + * @param int $stackPtr The stack position where the error occurred. + * @param string $code A violation code unique to the sniff message. + * @param array $data Replacements for the error message. + */ + protected function addError( $error, $stackPtr, $code, $data ) { + + if ( $this->is_inside_slashed_function_definition( $stackPtr ) ) { + $this->phpcsFile->addWarning( $error, $stackPtr, $code, $data ); + } else { + $this->phpcsFile->addError( $error, $stackPtr, $code, $data ); + } + } + + /** + * Determines whether or not a token is inside of a slashed function declaration. + * + * @since ${PROJECT_VERSION} + * + * @param int $stackPtr The position of the token in the stack. + * + * @return bool Whether the token is inside the definition of a slashed function. + */ + protected function is_inside_slashed_function_definition( $stackPtr ) { + + // Check if we are inside a function. + $function_ptr = $this->phpcsFile->getCondition( $stackPtr, T_FUNCTION ); + + if ( ! $function_ptr ) { + return false; + } + + $function_name = $this->phpcsFile->findNext( + Tokens::$emptyTokens, + $function_ptr + 1, + null, + true, + null, + true + ); + + $function_name = $this->tokens[ $function_name ]['content']; + + // If we are inside a function that expects slashed arguments we don't + // really need to flag any internal function calls, because if any of + // them expects slashed data but isn't receiving it, that is likely the + // reason that the wrapping function is in this list. + if ( + isset( self::$expectedSlashedFunctionArgs[ $function_name ] ) + || isset( self::$partlySlashedFunctionArgs[ $function_name ] ) + || isset( self::$mixedSlashedFunctionArgs[ $function_name ] ) + ) { + return true; + } + + return false; + } + } // End class. diff --git a/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc b/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc index 2012f98e05..a15bcdd28b 100644 --- a/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc +++ b/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc @@ -63,11 +63,11 @@ wp_list_pages( array( 'meta_value' => $value, 'link_before' => wp_slash( $before // Function declarations and function calls within the declaration of an ignored // function should both be ignored. function add_comment_meta( $comment_id, $meta_key, $meta_value, $unique = false ) { // OK - return add_metadata( 'comment', $comment_id, $meta_key, $meta_value, $unique ); // OK + return add_metadata( 'comment', $comment_id, $meta_key, $meta_value, $unique ); // Warning x 2 } function get_bookmarks( $r ) { // OK - return get_term_by( 'name', $r['category_name'], 'link_category' ); // OK + return get_term_by( 'name', $r['category_name'], 'link_category' ); // Warning } delete_metadata( 'user', 0, 'session_tokens', false, true ); // OK diff --git a/WordPress/Tests/WP/ExpectedSlashedUnitTest.php b/WordPress/Tests/WP/ExpectedSlashedUnitTest.php index b34b9edb01..c3593d353f 100644 --- a/WordPress/Tests/WP/ExpectedSlashedUnitTest.php +++ b/WordPress/Tests/WP/ExpectedSlashedUnitTest.php @@ -53,6 +53,7 @@ public function getWarningList() { 55 => 1, 56 => 1, 59 => 1, + 66 => 2, 70 => 1, ); } From ad4743944272b2cbc94d3bfff9cf197d8af46a3d Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Tue, 31 Oct 2017 17:10:08 -0400 Subject: [PATCH 06/21] Add more functions to lists --- WordPress/Sniffs/WP/ExpectedSlashedSniff.php | 141 ++++++++++++++++--- 1 file changed, 123 insertions(+), 18 deletions(-) diff --git a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php index 019b5ad212..bf385b2ad6 100644 --- a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php +++ b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php @@ -135,22 +135,19 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'wp_create_post_autosave' => array( 1 => 'post_data' ), // Uses wp_get_nav_menu_object(). 'wp_delete_nav_menu' => array( 1 => 'menu' ), + // Just passed data through it, but is used by wp_new_comment(), + // wp_update_comment(), etc. + 'wp_filter_comment' => array( 1 => 'commentarr' ), // Uses wp_get_nav_menu_object(). 'wp_get_nav_menu_items' => array( 1 => 'menu' ), // Uses get_term_by( 'name' ) if $menu is not a term ID or slug. 'wp_get_nav_menu_object' => array( 1 => 'menu' ), - // Uses wp_insert_post(). - 'wp_insert_attachment' => array( 1 => 'args' ), // Uses wp_unslash(). 'wp_insert_comment' => array( 1 => 'commentdata' ), // Uses wp_unslash(). 'wp_insert_link' => array( 1 => 'linkdata' ), // Uses wp_unslash(). - 'wp_insert_post' => array( 1 => 'postarr' ), - // Uses wp_unslash(). 'wp_insert_term' => array( 1 => 'term' ), - // Uses wp_unslash(), update_user_meta(). - 'wp_insert_user' => array( 1 => 'userdata' ), // Uses wp_insert_comment() and wp_allow_comment(). 'wp_new_comment' => array( 1 => 'commentdata' ), // Uses term_exists(). The docs for wp_remove_object_terms() says that it @@ -162,17 +159,16 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { // slugs or IDs, but it is also possible to pass in the term names, and in // that case they must be slashed. 'wp_set_object_terms' => array( 2 => 'terms' ), + // Uses wp_set_post_terms(). + 'wp_set_post_categories' => array( 2 => 'post_categories' ), + // Uses wp_set_post_terms(). + 'wp_set_post_tags' => array( 2 => 'tags' ), // Uses wp_set_object_terms(). 'wp_set_post_terms' => array( 2 => 'terms' ), // Uses update_post_meta(). 'wp_update_attachment_metadata' => array( 2 => 'data' ), // Uses wp_unslash(). 'wp_update_comment' => array( 1 => 'commentarr' ), - // Uses wp_insert_post(). If the $postarr is actually a post object and not - // an array, then it should be unslashed instead. - 'wp_update_post' => array( 1 => 'postarr' ), - // Uses wp_insert_user(). - 'wp_update_user' => array( 1 => 'userdata' ), // Uses install_blog(). 'wpmu_create_blog' => array( 3 => 'title' ), // Uses wp_unslash(). @@ -193,6 +189,24 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'pre_comment_author_email' => array( 1 => 'author_email_cookie' ), // Result passed through wp_unslash(). 'pre_comment_author_url' => array( 1 => 'author_url_cookie' ), + // Called in wp_filter_comment(). + 'pre_comment_content' => array( 1 => 'comment_content' ), + // Called in wp_filter_comment(). + 'pre_comment_user_agent' => array( 1 => 'comment_agent' ), + // Called in wp_insert_user(). + 'pre_user_description' => array( 1 => 'description' ), + // Called in wp_insert_user(). + 'pre_user_display_name' => array( 1 => 'display_name' ), + // Called in wp_insert_user(). + 'pre_user_first_name' => array( 1 => 'first_name' ), + // Called in wp_insert_user(). + 'pre_user_last_name' => array( 1 => 'last_name' ), + // Called in wp_insert_user(). + 'pre_user_email' => array( 1 => 'raw_user_email' ), + // Called in wp_insert_user(). + 'pre_user_nickname' => array( 1 => 'nickname' ), + // Called in wp_insert_user(). + 'pre_user_url' => array( 1 => 'raw_user_url' ), // Result passed through wp_unslash(). 'add_ping' => array( 1 => 'new' ), // Result passed to wp_list_pages(). @@ -240,7 +254,14 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { // Uses wp_unslash() on some of these. All of the other args are either // integers, slugs, or dates. 'wp_allow_comment' => array( - 1 => array( 'comment_author', 'comment_author_email', 'comment_author_url', 'comment_author_IP', 'comment_content', 'comment_agent' ), + 1 => array( + 'comment_author', + 'comment_author_email', + 'comment_author_url', + 'comment_author_IP', + 'comment_content', + 'comment_agent', + ), ), // Uses get_posts(). 'wp_get_nav_menu_items' => array( 2 => array( 's', 'title' ) ), @@ -248,6 +269,40 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'wp_get_post_revisions' => array( 2 => array( 's', 'title' ) ), // Uses get_posts(). 'wp_get_recent_posts' => array( 1 => array( 's', 'title' ) ), + // Uses wp_insert_post(). + 'wp_insert_attachment' => array( + 1 => array( + 'post_content', + 'post_content_filtered', + 'post_title', + 'post_excerpt', + 'post_password', + 'to_ping', + 'pinged', + 'guid', + 'post_category', + 'tags_input', + 'tax_input', + 'meta_input', + ), + ), + // Uses wp_unslash(). + 'wp_insert_post' => array( + 1 => array( + 'post_content', + 'post_content_filtered', + 'post_title', + 'post_excerpt', + 'post_password', + 'to_ping', + 'pinged', + 'guid', + 'post_category', + 'tags_input', + 'tax_input', + 'meta_input', + ), + ), // Uses wp_unslash() on this. All of the other args are integers or slugs. // The 'name' arg is also expected slashed, but this is always overridden by // $term. @@ -260,6 +315,24 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { // Uses get_term_by( 'name' ) with 'menu-name' and also passes the data to // wp_insert_term() if the menu doesn't exist, or else wp_update_term(). 'wp_update_nav_menu_object' => array( 2 => array( 'description', 'menu-name' ) ), + // Uses wp_insert_post(). If the $postarr is actually a post object and not + // an array, then it should be unslashed instead. + 'wp_update_post' => array( + 1 => array( + 'post_content', + 'post_content_filtered', + 'post_title', + 'post_excerpt', + 'post_password', + 'to_ping', + 'pinged', + 'guid', + 'post_category', + 'tags_input', + 'tax_input', + 'meta_input', + ), + ), // Uses wp_unslash() on these. All of the other args are integers or slugs. 'wp_update_term' => array( 3 => array( 'description', 'name' ) ), // Uses WP_Query::__construct(). @@ -332,6 +405,37 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'unslashed' => array( 'date_format', 'link_after', 'link_before', 'title_li' ), ), ), + // Uses wp_unslash(), but hashes the password first. Also uses + // update_user_meta(). + 'wp_insert_user' => array( + 1 => array( + 'slashed' => array( + 'description', + 'display_name', + 'first_name', + 'last_name', + 'nickname', + 'user_email', + 'user_url', + ), + 'unslashed' => array( 'user_pass' ), + ), + ), + // Uses wp_insert_user(). + 'wp_update_user' => array( + 1 => array( + 'slashed' => array( + 'description', + 'display_name', + 'first_name', + 'last_name', + 'nickname', + 'user_email', + 'user_url', + ), + 'unslashed' => array( 'user_pass' ), + ), + ), // 'wp_insert_category' => array( 3 => array( 'name', 'description' ) ), ); @@ -354,13 +458,14 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { * @var array */ public static $autoSlashingFunctions = array( - 'esc_url_raw' => true, - 'esc_url' => true, - 'get_current_user_id' => true, - 'sanitize_key' => true, - 'sanitize_title' => true, + 'esc_url_raw' => true, + 'esc_url' => true, + 'get_current_user_id' => true, + 'sanitize_key' => true, + 'sanitize_title' => true, 'sanitize_title_with_dashes' => true, - 'time' => true, + 'time' => true, + 'wp_filter_comment' => true, ); /** From 75c68d2a02e76cbb7934ad60fec0577b75e5d2d7 Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Tue, 31 Oct 2017 17:13:34 -0400 Subject: [PATCH 07/21] Move, rename, and alphabetize the list of filters --- WordPress/Sniffs/WP/ExpectedSlashedSniff.php | 76 +++++++++++--------- 1 file changed, 42 insertions(+), 34 deletions(-) diff --git a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php index bf385b2ad6..675ea78eab 100644 --- a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php +++ b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php @@ -181,40 +181,6 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'WP_Customize_Setting::sanitize' => array( 1 => 'value' ), ); - // TODO - public $filters = array( - // Result passed through wp_unslash(). - 'pre_comment_author_name' => array( 1 => 'author_cookie' ), - // Result passed through wp_unslash(). - 'pre_comment_author_email' => array( 1 => 'author_email_cookie' ), - // Result passed through wp_unslash(). - 'pre_comment_author_url' => array( 1 => 'author_url_cookie' ), - // Called in wp_filter_comment(). - 'pre_comment_content' => array( 1 => 'comment_content' ), - // Called in wp_filter_comment(). - 'pre_comment_user_agent' => array( 1 => 'comment_agent' ), - // Called in wp_insert_user(). - 'pre_user_description' => array( 1 => 'description' ), - // Called in wp_insert_user(). - 'pre_user_display_name' => array( 1 => 'display_name' ), - // Called in wp_insert_user(). - 'pre_user_first_name' => array( 1 => 'first_name' ), - // Called in wp_insert_user(). - 'pre_user_last_name' => array( 1 => 'last_name' ), - // Called in wp_insert_user(). - 'pre_user_email' => array( 1 => 'raw_user_email' ), - // Called in wp_insert_user(). - 'pre_user_nickname' => array( 1 => 'nickname' ), - // Called in wp_insert_user(). - 'pre_user_url' => array( 1 => 'raw_user_url' ), - // Result passed through wp_unslash(). - 'add_ping' => array( 1 => 'new' ), - // Result passed to wp_list_pages(). - 'widget_pages_args' => array( 1 => 'args' ), - // Result passed to wp_list_bookmarks(). - 'widget_links_args' => array( 1 => 'widget_links_args' ), - ); - /** * A list of functions and their args that are expected to be partly slashed. * @@ -439,6 +405,48 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { // 'wp_insert_category' => array( 3 => array( 'name', 'description' ) ), ); + /** + * The list of filters and their args that are expected to be slashed. + * + * @todo Sniff for these. + * + * @since ${PROJECT_VERSION} + * + * @var array + */ + public $expectedSlashedFilterArgs = array( + // Result passed through wp_unslash(). + 'add_ping' => array( 1 => 'new' ), + // Result passed through wp_unslash(). + 'pre_comment_author_email' => array( 1 => 'author_email_cookie' ), + // Result passed through wp_unslash(). + 'pre_comment_author_name' => array( 1 => 'author_cookie' ), + // Result passed through wp_unslash(). + 'pre_comment_author_url' => array( 1 => 'author_url_cookie' ), + // Called in wp_filter_comment(). + 'pre_comment_content' => array( 1 => 'comment_content' ), + // Called in wp_filter_comment(). + 'pre_comment_user_agent' => array( 1 => 'comment_agent' ), + // Called in wp_insert_user(). + 'pre_user_description' => array( 1 => 'description' ), + // Called in wp_insert_user(). + 'pre_user_display_name' => array( 1 => 'display_name' ), + // Called in wp_insert_user(). + 'pre_user_email' => array( 1 => 'raw_user_email' ), + // Called in wp_insert_user(). + 'pre_user_first_name' => array( 1 => 'first_name' ), + // Called in wp_insert_user(). + 'pre_user_last_name' => array( 1 => 'last_name' ), + // Called in wp_insert_user(). + 'pre_user_nickname' => array( 1 => 'nickname' ), + // Called in wp_insert_user(). + 'pre_user_url' => array( 1 => 'raw_user_url' ), + // Result passed to wp_list_bookmarks(). + 'widget_links_args' => array( 1 => 'widget_links_args' ), + // Result passed to wp_list_pages(). + 'widget_pages_args' => array( 1 => 'args' ), + ); + /** * Functions that slash data. * From 3004d53f442ae44dc9641c9378db2c78aea52862 Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Tue, 31 Oct 2017 17:14:54 -0400 Subject: [PATCH 08/21] Remove commented out code --- WordPress/Sniffs/WP/ExpectedSlashedSniff.php | 1 - 1 file changed, 1 deletion(-) diff --git a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php index 675ea78eab..8394af81ed 100644 --- a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php +++ b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php @@ -402,7 +402,6 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'unslashed' => array( 'user_pass' ), ), ), -// 'wp_insert_category' => array( 3 => array( 'name', 'description' ) ), ); /** From 4f305438caf5b381d28b8d0026d1fb59cc12b27b Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Tue, 31 Oct 2017 20:49:59 -0400 Subject: [PATCH 09/21] Warn about slashable static strings, and improve array value checks --- WordPress/Sniffs/WP/ExpectedSlashedSniff.php | 294 +++++++++++------- .../Tests/WP/ExpectedSlashedUnitTest.inc | 23 ++ .../Tests/WP/ExpectedSlashedUnitTest.php | 55 ++-- 3 files changed, 244 insertions(+), 128 deletions(-) diff --git a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php index 8394af81ed..3e88864a49 100644 --- a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php +++ b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php @@ -44,7 +44,6 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { * @var int[] */ protected $slashed_tokens = array( - T_CONSTANT_ENCAPSED_STRING => true, // TODO T_LNUMBER => true, T_MINUS => true, T_TRUE => true, @@ -646,25 +645,8 @@ protected function process_mixed_function_args( $parameters, $function_name ) { $tokens[ $array_opener ]['parenthesis_closer'] ) ) { - $is_slashed = false; - $start = $next_double_arrow + 1; - $value_ptr = $phpcsFile->findNext( - Tokens::$emptyTokens, - $next_double_arrow + 1, - null, - true, - null, - true - ); - - // These tokens are implicitly slashed, so we don't need to slash them. - if ( - isset( $this->slashed_tokens[ $tokens[ $value_ptr ]['code'] ] ) - || isset( $this->slashed_casts[ $tokens[ $value_ptr ]['code'] ] ) - ) { - continue; - } + $start = $next_double_arrow + 1; $key_ptr = $phpcsFile->findPrevious( Tokens::$emptyTokens, @@ -676,36 +658,46 @@ protected function process_mixed_function_args( $parameters, $function_name ) { ); if ( T_CONSTANT_ENCAPSED_STRING !== $tokens[ $key_ptr ]['code'] ) { + // TODO warning? continue; } $key_name = trim( $tokens[ $key_ptr ]['content'], '\'"' ); - if ( - isset( self::$slashingFunctions[ $tokens[ $value_ptr ]['content'] ] ) - || isset( self::$autoSlashingFunctions[ $tokens[ $value_ptr ]['content'] ] ) - ) { - $is_slashed = true; - } + $errors = array(); - if ( ! $is_slashed && in_array( $key_name, $slashed_keys, true ) ) { + if ( in_array( $key_name, $slashed_keys, true ) ) { - $this->addError( - '%s() expects the value of %s to be slashed with wp_slash().', - $value_ptr, - 'ExpectedKeySlashed', - array( $function_name, $key_name ) + $errors['unslashed'] = array( + 'message' => '%s() expects the value of %s to be slashed with wp_slash().', + 'code' => 'ExpectedKeySlashed', + 'data' => array( $function_name, $key_name ), ); - } elseif ( $is_slashed && in_array( $key_name, $unslashed_keys, true ) ) { + $errors['slashable_string'] = array( + 'message' => '%s() expects the value of %s to be slashed with wp_slash(); %s found.', + 'code' => 'KeyStringMayNeedSlashing', + 'data' => array( $function_name, $key_name ) + ); + + } elseif ( in_array( $key_name, $unslashed_keys, true ) ) { - $this->addError( - '%s() expects the value of %s to be unslashed.', - $value_ptr, - 'ExpectedKeyUnslashed', - array( $function_name, $key_name ) + $errors['slashed'] = array( + 'message' => '%s() expects the value of %s to be unslashed.', + 'code' => 'ExpectedKeyUnslashed', + 'data' => array( $function_name, $key_name ), ); } + + if ( empty( $errors ) ) { + continue; + } + + $this->slashing_check_loop( + $start, + $phpcsFile->findEndOfStatement( $start ), + $errors + ); } } @@ -722,7 +714,7 @@ protected function process_mixed_function_args( $parameters, $function_name ) { protected function process_expected_slashed_args( $parameters, $function_name ) { $phpcsFile = $this->phpcsFile; - $tokens = $this->tokens; + $tokens = $this->tokens; // Special handling for get_term_by( 'name' ). if ( 'get_term_by' === $function_name && isset( $parameters[1] ) ) { @@ -753,105 +745,195 @@ protected function process_expected_slashed_args( $parameters, $function_name ) break; } - $argPtr = $parameters[ $arg_index ]['start']; + $this->slashing_check_loop( + $parameters[ $arg_index ]['start'], + $parameters[ $arg_index ]['end'], + array( + 'unslashed' => array( + 'message' => '%s() expects the value of the $%s arg to be slashed with wp_slash(); %s found.', + 'code' => 'MissingSlashing', + 'data' => array( $function_name, $name ) + ), + 'slashable_string' => array( + 'message' => '%s() expects the value of the $%s arg to be slashed with wp_slash(); %s found.', + 'code' => 'StringMayNeedSlashing', + 'data' => array( $function_name, $name ) + ) + ) + ); + } - $in_cast = false; - $watch = true; + } // End process_expected_slashed_args() - for ( $i = $argPtr; $i <= $parameters[ $arg_index ]['end']; $i++ ) { + /** + * Loops over a series of tokens running a slashing check. + * + * @since ${PROJECT_VERSION} + * + * @param int $start The starting token position. + * @param int $end The ending token position. + * @param array $errors { + * The errors to give. + * + * @type array $slashed { + * The error to give when slashed data is found. + * + * @type string $message The error message. + * @type string $code The error code. + * @type array $data The error data. The found content will be added. + * } + * @type array $unslashed { + * The error to give when unslashed data is found. + * + * @type string $message The error message. + * @type string $code The error code. + * @type array $data The error data. The found content will be added. + * } + * @type array $slashable_string { + * The warning to give when a slashable string is found. + * + * @type string $message The error message. + * @type string $code The error code. + * @type array $data The error data. The found content will be added. + * } + * } + */ + protected function slashing_check_loop( $start, $end, $errors ) { - if ( T_COMMA === $tokens[ $i ]['code'] ) { + $tokens = $this->tokens; + + $in_cast = false; + $watch = true; + + for ( $i = $start; $i <= $end; $i++ ) { + + if ( T_COMMA === $tokens[ $i ]['code'] ) { + $watch = true; + continue; + } + + // If we're not watching right now, do nothing. + if ( ! $watch ) { + + // Wake up on concatenation characters, another part to check. + if ( T_STRING_CONCAT === $tokens[ $i ]['code'] ) { $watch = true; - continue; } - // If we're not watching right now, do nothing. - if ( ! $watch ) { + continue; + } - // Wake up on concatenation characters, another part to check. - if ( T_STRING_CONCAT === $tokens[ $i ]['code'] ) { - $watch = true; - } + // Ignore whitespaces and comments. + if ( in_array( $tokens[ $i ]['code'], Tokens::$emptyTokens ) ) { + continue; + } - continue; - } + // Skip to the end of a function call if it has been casted to a safe value. + if ( $in_cast && T_OPEN_PARENTHESIS === $tokens[ $i ]['code'] ) { + $i = $tokens[ $i ]['parenthesis_closer']; + $in_cast = false; + continue; + } - // Ignore whitespaces and comments. - if ( in_array( $tokens[ $i ]['code'], Tokens::$emptyTokens ) ) { - continue; - } + // Handle arrays for those functions that accept them. + if ( T_ARRAY === $tokens[ $i ]['code'] ) { + $i++; // Skip the opening parenthesis. + continue; + } - // Skip to the end of a function call if it has been casted to a safe value. - if ( $in_cast && T_OPEN_PARENTHESIS === $tokens[ $i ]['code'] ) { - $i = $tokens[ $i ]['parenthesis_closer']; - $in_cast = false; - continue; - } + if ( in_array( $tokens[ $i ]['code'], array( T_DOUBLE_ARROW, T_CLOSE_PARENTHESIS, T_STRING_CONCAT ) ) ) { + continue; + } - // Handle arrays for those functions that accept them. - if ( T_ARRAY === $tokens[ $i ]['code'] ) { - $i ++; // Skip the opening parenthesis. - continue; - } + // Allow tokens that are implicitly slashed. + if ( isset( $this->slashed_tokens[ $tokens[ $i ]['code'] ] ) ) { + continue; + } - if ( in_array( $tokens[ $i ]['code'], array( T_DOUBLE_ARROW, T_CLOSE_PARENTHESIS, T_STRING_CONCAT ) ) ) { - continue; - } + $content = $tokens[ $i ]['content']; - // Allow tokens that are implicitly slashed. - if ( isset( $this->slashed_tokens[ $tokens[ $i ]['code'] ] ) ) { - continue; + if ( T_CONSTANT_ENCAPSED_STRING === $tokens[ $i ]['code'] ) { + + if ( preg_match( '~^["\'].*["\'\\\\].*["\']$~', $content ) ) { + if ( isset( $errors['slashable_string'] ) ) { + $this->phpcsFile->addWarning( + $errors['slashable_string']['message'], + $i, + $errors['slashable_string']['code'], + array_merge( $errors['slashable_string']['data'], array( $content ) ) + ); + } } - // If we were watching before, stop now. That way we'll error once - // for a set of unslashed tokens, instead of for each of the tokens. - $watch = false; + continue; + } - // Allow int/float/bool casted variables. - if ( isset( $this->slashed_casts[ $tokens[ $i ]['code'] ] ) ) { - $in_cast = true; - continue; - } + // If we were watching before, stop now. That way we'll error once + // for a set of unslashed tokens, instead of for each of the tokens. + $watch = false; - $content = $tokens[ $i ]['content']; + // Allow int/float/bool casted variables. + if ( isset( $this->slashed_casts[ $tokens[ $i ]['code'] ] ) ) { + $in_cast = true; + continue; + } - // If this is a function call. - if ( T_STRING === $tokens[ $i ]['code'] ) { + // If this is a function call. + if ( T_STRING === $tokens[ $i ]['code'] ) { - // We can fast-forward to the end of this function call, we don't - // need to check each token because we're going to check whether - // the result of the function is slashed (below). - $paren_opener = $phpcsFile->findNext( Tokens::$emptyTokens, $i + 1, null, true ); + // We can fast-forward to the end of this function call, we don't + // need to check each token because we're going to check whether + // the result of the function is slashed (below). + $paren_opener = $this->phpcsFile->findNext( Tokens::$emptyTokens, $i + 1, null, true ); - if ( isset( $tokens[ $paren_opener ]['parenthesis_closer'] ) ) { - $i = $tokens[ $paren_opener ]['parenthesis_closer']; - } + if ( isset( $tokens[ $paren_opener ]['parenthesis_closer'] ) ) { + $i = $tokens[ $paren_opener ]['parenthesis_closer']; + } - // If the function is a slashing function we continue to the next - // token instead of giving an error. - if ( - isset( self::$slashingFunctions[ $content ] ) - || isset( self::$autoSlashingFunctions[ $content ] ) - ) { - continue; + // If the function is a slashing function we continue to the next + // token instead of giving an error. + if ( + isset( self::$slashingFunctions[ $content ] ) + || isset( self::$autoSlashingFunctions[ $content ] ) + ) { + if ( isset( $errors['slashed'] ) ) { + $this->addError( + $errors['slashed']['message'], + $i, + $errors['slashed']['code'], + array_merge( $errors['slashed']['data'], array( $content ) ) + ); } - } elseif ( T_VARIABLE === $tokens[ $i ]['code'] ) { - if ( in_array( $content, $this->input_superglobals, true ) ) { - continue; + continue; + } + + } elseif ( T_VARIABLE === $tokens[ $i ]['code'] ) { + if ( in_array( $content, $this->input_superglobals, true ) ) { + + if ( isset( $errors['slashed'] ) ) { + $this->addError( + $errors['slashed']['message'], + $i, + $errors['slashed']['code'], + array_merge( $errors['slashed']['data'], array( $content ) ) + ); } + + continue; } + } + if ( isset( $errors['unslashed'] ) ) { $this->addError( - '%s() expects the value of the $%s arg to be slashed with wp_slash(); %s found.', + $errors['unslashed']['message'], $i, - 'MissingSlashing', - array( $function_name, $name, $content ) + $errors['unslashed']['code'], + array_merge( $errors['unslashed']['data'], array( $content ) ) ); } } - - } // End process_expected_slashed_args() + } /** * Records an error against a specific token in the file being sniffed. diff --git a/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc b/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc index a15bcdd28b..7f16caa34f 100644 --- a/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc +++ b/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc @@ -97,3 +97,26 @@ wpmu_validate_blog_signup( $_POST['blogname'], $_POST['blog_title'], $user ); // // Without whitespace. get_bookmarks(array("category" => $cat->term_id)); // OK. + +// String passed. +add_post_meta( $post_id, 'key', 'value' ); // OK. +add_post_meta( $post_id, 'key', 'I said, "Wow!"' ); // Warning. +add_post_meta( $post_id, 'key', 'That\'s nice.' ); // Warning. +add_post_meta( $post_id, 'key', 'Slashes (\\).' ); // Warning. + +wp_update_term( $term_id, 'tag', array( 'name' => 'value' ) ); // OK. +wp_update_term( $term_id, 'tag', array( 'name' => 'I said, "Wow!"' ) ); // Warning. + +wp_list_pages( array( 'meta_value' => 'value' ) ); // OK. +wp_list_pages( array( 'meta_value' => 'I said, "Wow!"' ) ); // Warning. + +// Mixed. +add_post_meta( $post_id, 'key', wp_slash( $value ) . $value . ' "' ); // 1 Error, 1 Warning. + +wp_update_term( $term_id, 'tag', array( 'name' => wp_slash( $value ) . $value . ' "' ) ); // 1 Error, 1 Warning. + +wp_list_pages( array( 'meta_value' => wp_slash( $value ) . $value . ' "' ) ); // 1 Error, 1 Warning. + +// More slashed superglobals. +wp_update_term( $term_id, 'tag', array( 'name' => $_POST['something'] ) ); // OK. +wp_list_pages( array( 'meta_value' => $_POST['something'] ) ); // OK. diff --git a/WordPress/Tests/WP/ExpectedSlashedUnitTest.php b/WordPress/Tests/WP/ExpectedSlashedUnitTest.php index c3593d353f..125ca6dbdf 100644 --- a/WordPress/Tests/WP/ExpectedSlashedUnitTest.php +++ b/WordPress/Tests/WP/ExpectedSlashedUnitTest.php @@ -25,23 +25,26 @@ class ExpectedSlashedUnitTest extends AbstractSniffUnitTest { */ public function getErrorList() { return array( - 6 => 1, - 9 => 1, - 16 => 1, - 17 => 1, - 19 => 1, - 22 => 1, - 31 => 2, - 32 => 1, - 36 => 1, - 41 => 1, - 44 => 1, - 45 => 1, - 57 => 1, - 60 => 1, - 61 => 2, - 92 => 1, - 93 => 1, + 6 => 1, + 9 => 1, + 16 => 1, + 17 => 1, + 19 => 1, + 22 => 1, + 31 => 2, + 32 => 1, + 36 => 1, + 41 => 1, + 44 => 1, + 45 => 1, + 57 => 1, + 60 => 1, + 61 => 2, + 92 => 1, + 93 => 1, + 114 => 1, + 116 => 1, + 118 => 1, ); } @@ -50,11 +53,19 @@ public function getErrorList() { */ public function getWarningList() { return array( - 55 => 1, - 56 => 1, - 59 => 1, - 66 => 2, - 70 => 1, + 55 => 1, + 56 => 1, + 59 => 1, + 66 => 2, + 70 => 1, + 103 => 1, + 104 => 1, + 105 => 1, + 108 => 1, + 111 => 1, + 114 => 1, + 116 => 1, + 118 => 1, ); } From e45441844b5091d8f68f247a1fed2c17dfab84f1 Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Wed, 1 Nov 2017 10:06:42 -0400 Subject: [PATCH 10/21] Add another function --- WordPress/Sniffs/WP/ExpectedSlashedSniff.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php index 3e88864a49..3ea8591b7c 100644 --- a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php +++ b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php @@ -128,6 +128,8 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'update_user_option' => array( 2 => 'option_name', 3 => 'newvalue' ), // Uses wp_set_object_terms(). 'wp_add_object_terms' => array( 2 => 'terms' ), + // Uses wp_set_post_tags(). + 'wp_add_post_tags' => array( 2 => 'tags' ), // Uses wp_update_nav_menu_object(). 'wp_create_nav_menu' => array( 1 => 'menu_name' ), // Uses wp_unslash(). From 3318f31145b252664c34248da0961337c1b6a455 Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Wed, 1 Nov 2017 10:07:49 -0400 Subject: [PATCH 11/21] Don't assume whitespace for get_term_by() --- WordPress/Sniffs/WP/ExpectedSlashedSniff.php | 2 +- WordPress/Tests/WP/ExpectedSlashedUnitTest.inc | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php index 3ea8591b7c..af22b7872e 100644 --- a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php +++ b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php @@ -723,7 +723,7 @@ protected function process_expected_slashed_args( $parameters, $function_name ) $byPtr = $phpcsFile->findNext( Tokens::$emptyTokens, - $parameters[1]['start'] + 1, + $parameters[1]['start'], null, true, null, diff --git a/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc b/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc index 7f16caa34f..f561e7f4ad 100644 --- a/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc +++ b/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc @@ -120,3 +120,6 @@ wp_list_pages( array( 'meta_value' => wp_slash( $value ) . $value . ' "' ) ); // // More slashed superglobals. wp_update_term( $term_id, 'tag', array( 'name' => $_POST['something'] ) ); // OK. wp_list_pages( array( 'meta_value' => $_POST['something'] ) ); // OK. + +// Without spaces. +get_term_by('slug', 'post-format-' . $value, 'post_format' ); From f9192a6bdc1125ae978deadce656ddbe83c264af Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Wed, 1 Nov 2017 12:07:08 -0400 Subject: [PATCH 12/21] Add more functions --- WordPress/Sniffs/WP/ExpectedSlashedSniff.php | 26 ++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php index af22b7872e..e598a1767c 100644 --- a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php +++ b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php @@ -71,6 +71,8 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'add_term_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), // Uses add_metadata(). 'add_user_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + // Uses term_exists(). + 'category_exists' => array( 1 => 'cat_name' ), // These are directly interpolated into a database query. Failure to slash // will result in SQL injection!! 'check_comment' => array( 1 => 'author', 2 => 'email' ), @@ -94,6 +96,10 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'edit_post' => array( 1 => 'post_data' ), // Uses get_term_by( 'name' ). 'get_cat_ID' => array( 1 => 'category_name' ), + // Uses get_term_by( 'name' ). + 'get_linkobjectsbyname' => array( 1 => 'cat_name' ), + // Uses get_term_by( 'name' ). + 'get_linksbyname' => array( 1 => 'cat_name' ), // Uses get_search_feed_link(). 'get_search_comments_feed_link' => array( 1 => array( 'search_query' ) ), // Uses get_search_link(). @@ -106,6 +112,8 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'install_blog' => array( 2 => 'blog_title' ), // Uses wp_get_nav_menu_object(). 'is_nav_menu' => array( 1 => 'menu' ), + // Uses term_exists(). + 'is_term' => array( 1 => 'term' ), // Uses wp_unslash(). 'post_exists' => array( 1 => 'title', 2 => 'content', 3 => 'date' ), // Uses update_post_meta() when the $file isn't empty. @@ -122,6 +130,8 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'update_term_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), // Uses update_metadata(). 'update_user_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + // Uses term_exists(). + 'tag_exists' => array( 1 => 'tag_name' ), // Uses wp_unslash() when a string is passed; also accepts term ID. 'term_exists' => array( 1 => 'term' ), // Uses update_user_meta(). @@ -134,6 +144,8 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'wp_create_nav_menu' => array( 1 => 'menu_name' ), // Uses wp_unslash(). 'wp_create_post_autosave' => array( 1 => 'post_data' ), + // Uses wp_insert_term() and term_exists(). + 'wp_create_term' => array( 1 => 'term_name' ), // Uses wp_get_nav_menu_object(). 'wp_delete_nav_menu' => array( 1 => 'menu' ), // Just passed data through it, but is used by wp_new_comment(), @@ -162,6 +174,8 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'wp_set_object_terms' => array( 2 => 'terms' ), // Uses wp_set_post_terms(). 'wp_set_post_categories' => array( 2 => 'post_categories' ), + // Uses wp_set_post_categories(). + 'wp_set_post_cats' => array( 2 => 'post_categories' ), // Uses wp_set_post_terms(). 'wp_set_post_tags' => array( 2 => 'tags' ), // Uses wp_set_object_terms(). @@ -180,6 +194,12 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'WP_Press_This::side_load_images' => array( 2 => 'content' ), // Uses wp_unslash(). 'WP_Customize_Setting::sanitize' => array( 1 => 'value' ), + // Uses wp_insert_post() and wp_update_post(). + 'wp_xmlrpc_server::_insert_post' => array( 2 => 'content_struct' ), + // Uses wp_unslash() and add_term_meta(). + 'wp_xmlrpc_server::set_term_custom_fields' => array( 2 => 'fields' ), + // Uses wp_unslash() and add_post_meta(). + 'wp_xmlrpc_server::set_custom_fields' => array( 2 => 'fields' ), ); /** @@ -253,6 +273,8 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'meta_input', ), ), + // Uses wp_insert_term() and wp_update_term(). + 'wp_insert_category' => array( 1 => array( 'category_description', 'cat_name' ) ), // Uses wp_unslash(). 'wp_insert_post' => array( 1 => array( @@ -274,6 +296,8 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { // The 'name' arg is also expected slashed, but this is always overridden by // $term. 'wp_insert_term' => array( 3 => array( 'description' ) ), + // Uses wp_get_nav_menu_object() on this. + 'wp_nav_menu' => array( 1 => array( 'menu' ) ), // Uses wp_insert_post() or wp_update_post(). All other values are slugs or // integers. 'wp_update_nav_menu_item' => array( @@ -445,6 +469,8 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'widget_links_args' => array( 1 => 'widget_links_args' ), // Result passed to wp_list_pages(). 'widget_pages_args' => array( 1 => 'args' ), + // Result passed to wp_update_post() or wp_insert_post(). + 'xmlrpc_wp_insert_post_data' => array( 1 => 'post_data', 2 => 'content_struct' ), ); /** From a4162b3e05e6039ae84fbb94f10ed055ad98c5a5 Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Wed, 1 Nov 2017 12:13:09 -0400 Subject: [PATCH 13/21] Better handling of partly-slashed functions when not arrays --- WordPress/Sniffs/WP/ExpectedSlashedSniff.php | 23 +++++++++---------- .../Tests/WP/ExpectedSlashedUnitTest.inc | 3 +++ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php index e598a1767c..09a6f57a9b 100644 --- a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php +++ b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php @@ -633,18 +633,17 @@ protected function process_mixed_function_args( $parameters, $function_name ) { } else { - if ( - ! isset( self::$slashingFunctions[ $tokens[ $arg_start ]['content'] ] ) - && ! isset( self::$autoSlashingFunctions[ $tokens[ $arg_start ]['content'] ] ) - ) { - - $this->addError( - '%s() expects the value of %s to be slashed with wp_slash().', - $argPtr, - 'ExpectedPartlySlashed', - array( $function_name, implode( ', ', $slashed_keys ) ) - ); - } + $this->slashing_check_loop( + $arg_start, + $parameters[ $arg_index ]['end'], + array( + 'unslashed' => array( + 'message' => '%s() expects the value of %s to be slashed with wp_slash().', + 'code' => 'ExpectedPartlySlashed', + 'data' => array( $function_name, implode( ', ', $slashed_keys ) ), + ), + ) + ); } break; diff --git a/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc b/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc index f561e7f4ad..3ea1eb3dbc 100644 --- a/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc +++ b/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc @@ -123,3 +123,6 @@ wp_list_pages( array( 'meta_value' => $_POST['something'] ) ); // OK. // Without spaces. get_term_by('slug', 'post-format-' . $value, 'post_format' ); + +// Slashed superglobals. +wp_insert_post( $_POST ); From 49b33eeefcaadd19c547f25e367d9a6c2c6f04fb Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Wed, 1 Nov 2017 15:00:07 -0400 Subject: [PATCH 14/21] Add another function --- WordPress/Sniffs/WP/ExpectedSlashedSniff.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php index 09a6f57a9b..3653000efe 100644 --- a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php +++ b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php @@ -226,6 +226,23 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { public static $partlySlashedFunctionArgs = array( // Uses query_posts(), 'q' is passed as 's'. '_wp_ajax_menu_quick_search' => array( 1 => array( 'q' ) ), + // Uses wp_update_post(). + 'bulk_edit_posts' => array( + 1 => array( + 'post_content', + 'post_content_filtered', + 'post_title', + 'post_excerpt', + 'post_password', + 'to_ping', + 'pinged', + 'guid', + 'post_category', + 'tags_input', + 'tax_input', + 'meta_input', + ), + ), // Uses get_posts() with the $args if they are an array. 'get_children' => array( 1 => array( 's', 'title' ) ), // Uses get_term_by( 'name' ). All of the other args are either integers or From 0655848da8625ec3a950defc90fd138b1d1ce265 Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Wed, 1 Nov 2017 15:05:40 -0400 Subject: [PATCH 15/21] Always include found value in error messages --- WordPress/Sniffs/WP/ExpectedSlashedSniff.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php index 3653000efe..281d6ae7e1 100644 --- a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php +++ b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php @@ -638,13 +638,14 @@ protected function process_mixed_function_args( $parameters, $function_name ) { if ( $is_mixed ) { $phpcsFile->addWarning( - '%s() expects the value of %s to be slashed with wp_slash(), and %s to be unslashed.', + '%s() expects the value of %s to be slashed with wp_slash(), and %s to be unslashed; %s found.', $argPtr, 'ExpectedMixed', array( $function_name, implode( ', ', $slashed_keys ), - implode( ', ', $unslashed_keys ) + implode( ', ', $unslashed_keys ), + $tokens[ $arg_start ]['content'] ) ); @@ -655,7 +656,7 @@ protected function process_mixed_function_args( $parameters, $function_name ) { $parameters[ $arg_index ]['end'], array( 'unslashed' => array( - 'message' => '%s() expects the value of %s to be slashed with wp_slash().', + 'message' => '%s() expects the value of %s to be slashed with wp_slash(); %s found.', 'code' => 'ExpectedPartlySlashed', 'data' => array( $function_name, implode( ', ', $slashed_keys ) ), ), @@ -713,7 +714,7 @@ protected function process_mixed_function_args( $parameters, $function_name ) { if ( in_array( $key_name, $slashed_keys, true ) ) { $errors['unslashed'] = array( - 'message' => '%s() expects the value of %s to be slashed with wp_slash().', + 'message' => '%s() expects the value of %s to be slashed with wp_slash(); %s found.', 'code' => 'ExpectedKeySlashed', 'data' => array( $function_name, $key_name ), ); @@ -727,7 +728,7 @@ protected function process_mixed_function_args( $parameters, $function_name ) { } elseif ( in_array( $key_name, $unslashed_keys, true ) ) { $errors['slashed'] = array( - 'message' => '%s() expects the value of %s to be unslashed.', + 'message' => '%s() expects the value of %s to be unslashed; %s found.', 'code' => 'ExpectedKeyUnslashed', 'data' => array( $function_name, $key_name ), ); From c32a65318002b6fb69e8ef746c0a957dbdfcc675 Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Wed, 1 Nov 2017 16:54:05 -0400 Subject: [PATCH 16/21] Add more functions --- WordPress/Sniffs/WP/ExpectedSlashedSniff.php | 96 +++++++++++++++++++- 1 file changed, 95 insertions(+), 1 deletion(-) diff --git a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php index 281d6ae7e1..1c568b8505 100644 --- a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php +++ b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php @@ -100,6 +100,8 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'get_linkobjectsbyname' => array( 1 => 'cat_name' ), // Uses get_term_by( 'name' ). 'get_linksbyname' => array( 1 => 'cat_name' ), + // Uses get_linksbyname(). + 'get_linksbyname_withrating' => array( 1 => 'cat_name' ), // Uses get_search_feed_link(). 'get_search_comments_feed_link' => array( 1 => array( 'search_query' ) ), // Uses get_search_link(). @@ -140,10 +142,16 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'wp_add_object_terms' => array( 2 => 'terms' ), // Uses wp_set_post_tags(). 'wp_add_post_tags' => array( 2 => 'tags' ), + // Uses category_exists() and wp_create_category(). + 'wp_create_categories' => array( 1 => 'categories' ), + // Uses category_exists() and wp_insert_category(). + 'wp_create_category' => array( 1 => 'cat_name' ), // Uses wp_update_nav_menu_object(). 'wp_create_nav_menu' => array( 1 => 'menu_name' ), // Uses wp_unslash(). 'wp_create_post_autosave' => array( 1 => 'post_data' ), + // Uses wp_create_term(). + 'wp_create_tag' => array( 1 => 'tag_name' ), // Uses wp_insert_term() and term_exists(). 'wp_create_term' => array( 1 => 'term_name' ), // Uses wp_get_nav_menu_object(). @@ -153,7 +161,7 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'wp_filter_comment' => array( 1 => 'commentarr' ), // Uses wp_get_nav_menu_object(). 'wp_get_nav_menu_items' => array( 1 => 'menu' ), - // Uses get_term_by( 'name' ) if $menu is not a term ID or slug. + // Uses get_term_by( 'name' ) if $menu is not a term object, ID, or slug. 'wp_get_nav_menu_object' => array( 1 => 'menu' ), // Uses wp_unslash(). 'wp_insert_comment' => array( 1 => 'commentdata' ), @@ -184,6 +192,8 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'wp_update_attachment_metadata' => array( 2 => 'data' ), // Uses wp_unslash(). 'wp_update_comment' => array( 1 => 'commentarr' ), + // Uses wp_insert_link(). + 'wp_update_link' => array( 1 => 'linkdata' ), // Uses install_blog(). 'wpmu_create_blog' => array( 3 => 'title' ), // Uses wp_unslash(). @@ -253,6 +263,40 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'get_pages' => array( 1 => array( 'meta_key', 'meta_value' ) ), // Uses WP_Query::query(). 'get_posts' => array( 1 => array( 's', 'title' ) ), + // Uses wp_insert_attachment(). + 'media_handle_sideload' => array( + 4 => array( + 'post_content', + 'post_content_filtered', + 'post_title', + 'post_excerpt', + 'post_password', + 'to_ping', + 'pinged', + 'guid', + 'post_category', + 'tags_input', + 'tax_input', + 'meta_input', + ), + ), + // Uses wp_insert_attachment(). + 'media_handle_upload' => array( + 4 => array( + 'post_content', + 'post_content_filtered', + 'post_title', + 'post_excerpt', + 'post_password', + 'to_ping', + 'pinged', + 'guid', + 'post_category', + 'tags_input', + 'tax_input', + 'meta_input', + ), + ), // Uses WP_Query::query(). 'query_posts' => array( 1 => array( 's', 'title' ) ), // Uses wp_unslash() on some of these. All of the other args are either @@ -315,6 +359,12 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'wp_insert_term' => array( 3 => array( 'description' ) ), // Uses wp_get_nav_menu_object() on this. 'wp_nav_menu' => array( 1 => array( 'menu' ) ), + // Uses wp_update_nav_menu_item(). + 'wp_save_nav_menu_items' => array( + 2 => array( 'menu-item-description', 'menu-item-attr-title', 'menu-item-title' ) + ), + // Uses wp_insert_category(). + 'wp_update_category' => array( 1 => array( 'category_description', 'cat_name' ) ), // Uses wp_insert_post() or wp_update_post(). All other values are slugs or // integers. 'wp_update_nav_menu_item' => array( @@ -343,6 +393,23 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { ), // Uses wp_unslash() on these. All of the other args are integers or slugs. 'wp_update_term' => array( 3 => array( 'description', 'name' ) ), + // Uses wp_insert_attachment(). + 'Custom_Image_Header::insert_attachment' => array( + 1 => array( + 'post_content', + 'post_content_filtered', + 'post_title', + 'post_excerpt', + 'post_password', + 'to_ping', + 'pinged', + 'guid', + 'post_category', + 'tags_input', + 'tax_input', + 'meta_input', + ), + ), // Uses WP_Query::__construct(). 'WP_Customize_Nav_Menus::search_available_items_query' => array( 1 => array( 's' ) ), // Uses WP_Query::query(). @@ -355,6 +422,23 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { // WP_Query::parse_search() uses stripslashes() on 's'. All other args are // either integers, booleans, or slug-like strings. 'WP_Query::query' => array( 1 => array( 's', 'title' ) ), + // Uses wp_insert_attachment(). + 'WP_Site_Icon::insert_attachment' => array( + 1 => array( + 'post_content', + 'post_content_filtered', + 'post_title', + 'post_excerpt', + 'post_password', + 'to_ping', + 'pinged', + 'guid', + 'post_category', + 'tags_input', + 'tax_input', + 'meta_input', + ), + ), // Uses WP_Query::query(). '_WP_Editors::wp_link_query' => array( 1 => array( 's' ) ), ); @@ -458,6 +542,12 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { public $expectedSlashedFilterArgs = array( // Result passed through wp_unslash(). 'add_ping' => array( 1 => 'new' ), + // Passed POSTed data. + 'attachment_fields_to_save' => array( 1 => 'post', 2 => 'attachment' ), + // Result passed to wp_insert_attachment(). + 'attachment_thumbnail_args' => array( 1 => 'image_attachment' ), + // Result passed to get_posts(). + 'dashboard_recent_drafts_query_args' => array( 1 => 'query_args' ), // Result passed through wp_unslash(). 'pre_comment_author_email' => array( 1 => 'author_email_cookie' ), // Result passed through wp_unslash(). @@ -482,6 +572,8 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'pre_user_nickname' => array( 1 => 'nickname' ), // Called in wp_insert_user(). 'pre_user_url' => array( 1 => 'raw_user_url' ), + // Passed POSTed data. + 'set-screen-option' => array( 1 => 'value', 2 => 'option', 3 => 'value' ), // Result passed to wp_list_bookmarks(). 'widget_links_args' => array( 1 => 'widget_links_args' ), // Result passed to wp_list_pages(). @@ -509,9 +601,11 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { * @var array */ public static $autoSlashingFunctions = array( + 'absint' => true, 'esc_url_raw' => true, 'esc_url' => true, 'get_current_user_id' => true, + 'intval' => true, 'sanitize_key' => true, 'sanitize_title' => true, 'sanitize_title_with_dashes' => true, From 2c18b122be8fc857532cf5becf01cda5286402b3 Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Wed, 1 Nov 2017 17:07:25 -0400 Subject: [PATCH 17/21] Give a warning when the key is not a string --- WordPress/Sniffs/WP/ExpectedSlashedSniff.php | 23 ++++++++++++++++++- .../Tests/WP/ExpectedSlashedUnitTest.inc | 5 ++++ .../Tests/WP/ExpectedSlashedUnitTest.php | 2 ++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php index 1c568b8505..191063c711 100644 --- a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php +++ b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php @@ -797,7 +797,28 @@ protected function process_mixed_function_args( $parameters, $function_name ) { ); if ( T_CONSTANT_ENCAPSED_STRING !== $tokens[ $key_ptr ]['code'] ) { - // TODO warning? + + $data = array( + $function_name, + implode( ', ', $slashed_keys ), + implode( ', ', $unslashed_keys ), + $tokens[ $key_ptr ]['content'] + ); + + if ( $is_mixed ) { + $message = '%s() expects the value of %s to be slashed with wp_slash(), and %s to be unslashed; unknown key "%s" found.'; + } else { + $message = '%s() expects the value of %s to be slashed with wp_slash(); unknown key "%s" found.'; + unset( $data[2] ); + } + + $phpcsFile->addWarning( + $message, + $argPtr, + 'MaybeExpectedKeySlashed', + $data + ); + continue; } diff --git a/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc b/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc index 3ea1eb3dbc..da8d50d5f5 100644 --- a/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc +++ b/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc @@ -126,3 +126,8 @@ get_term_by('slug', 'post-format-' . $value, 'post_format' ); // Slashed superglobals. wp_insert_post( $_POST ); + +// Unknown key. +wp_update_term( $term_id, 'tag', array( $value => 'value' ) ); // Warning. + +wp_list_pages( array( $value => 'value' ) ); // Warning. diff --git a/WordPress/Tests/WP/ExpectedSlashedUnitTest.php b/WordPress/Tests/WP/ExpectedSlashedUnitTest.php index 125ca6dbdf..1cd96740d2 100644 --- a/WordPress/Tests/WP/ExpectedSlashedUnitTest.php +++ b/WordPress/Tests/WP/ExpectedSlashedUnitTest.php @@ -66,6 +66,8 @@ public function getWarningList() { 114 => 1, 116 => 1, 118 => 1, + 131 => 1, + 133 => 1, ); } From 79900c6c85108408610bf6d9fe0736b938505f4c Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Fri, 3 Nov 2017 17:03:37 -0400 Subject: [PATCH 18/21] Update function lists --- WordPress/Sniffs/WP/ExpectedSlashedSniff.php | 109 ++++++++++++++----- 1 file changed, 83 insertions(+), 26 deletions(-) diff --git a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php index 191063c711..62350855e7 100644 --- a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php +++ b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php @@ -73,11 +73,10 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'add_user_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), // Uses term_exists(). 'category_exists' => array( 1 => 'cat_name' ), - // These are directly interpolated into a database query. Failure to slash - // will result in SQL injection!! + // Uses wp_unslash(). 'check_comment' => array( 1 => 'author', 2 => 'email' ), // Uses stripslashes(). - 'comment_exists' => array( 1 => 'comment_author' ), + 'comment_exists' => array( 1 => 'comment_author', 2 => 'comment_date' ), // Uses delete_metadata(). 'delete_comment_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), // Uses wp_unslash(). @@ -94,6 +93,9 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'delete_user_option' => array( 2 => 'option_name' ), // Expects POST data. 'edit_post' => array( 1 => 'post_data' ), + // Uses stripslashes(), but adds slashes back with addslashes() (which is + // necessary escaping for the context the output is intended to be used in). + 'esc_js' => array( 1 => 'text' ), // Uses get_term_by( 'name' ). 'get_cat_ID' => array( 1 => 'category_name' ), // Uses get_term_by( 'name' ). @@ -116,6 +118,8 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'is_nav_menu' => array( 1 => 'menu' ), // Uses term_exists(). 'is_term' => array( 1 => 'term' ), + // Uses esc_js(). + 'js_escape' => array( 1 => 'text' ), // Uses wp_unslash(). 'post_exists' => array( 1 => 'title', 2 => 'content', 3 => 'date' ), // Uses update_post_meta() when the $file isn't empty. @@ -132,6 +136,8 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'update_term_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), // Uses update_metadata(). 'update_user_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + // Uses stripslashes(). + 'update_usermeta' => array( 3 => 'meta_value' ), // Uses term_exists(). 'tag_exists' => array( 1 => 'tag_name' ), // Uses wp_unslash() when a string is passed; also accepts term ID. @@ -159,6 +165,12 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { // Just passed data through it, but is used by wp_new_comment(), // wp_update_comment(), etc. 'wp_filter_comment' => array( 1 => 'commentarr' ), + // Uses stripslashes(), but adds slashes back with addslashes(). + 'wp_filter_kses' => array( 1 => 'data' ), + // Uses stripslashes(), but adds slashes back with addslashes(). + 'wp_filter_nohtml_kses' => array( 1 => 'data' ), + // Uses stripslashes(), but adds slashes back with addslashes(). + 'wp_filter_post_kses' => array( 1 => 'data' ), // Uses wp_get_nav_menu_object(). 'wp_get_nav_menu_items' => array( 1 => 'menu' ), // Uses get_term_by( 'name' ) if $menu is not a term object, ID, or slug. @@ -171,6 +183,8 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'wp_insert_term' => array( 1 => 'term' ), // Uses wp_insert_comment() and wp_allow_comment(). 'wp_new_comment' => array( 1 => 'commentdata' ), + // Uses stripslashes(), but adds the slashes back with addslashes(). + 'wp_rel_nofollow' => array( 1 => 'text' ), // Uses term_exists(). The docs for wp_remove_object_terms() says that it // takes only term slugs or IDs, but it is also possible to pass in the term // names, and in that case they must be slashed. @@ -204,6 +218,8 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'WP_Press_This::side_load_images' => array( 2 => 'content' ), // Uses wp_unslash(). 'WP_Customize_Setting::sanitize' => array( 1 => 'value' ), + // Uses wp_unslash(). + 'WP_User_Search::__construct' => array( 1 => 'search_term' ), // Uses wp_insert_post() and wp_update_post(). 'wp_xmlrpc_server::_insert_post' => array( 2 => 'content_struct' ), // Uses wp_unslash() and add_term_meta(). @@ -234,7 +250,7 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { * @var array[] */ public static $partlySlashedFunctionArgs = array( - // Uses query_posts(), 'q' is passed as 's'. + // Uses WP_Query::__construct(), 'q' is passed as 's'. '_wp_ajax_menu_quick_search' => array( 1 => array( 'q' ) ), // Uses wp_update_post(). 'bulk_edit_posts' => array( @@ -253,16 +269,12 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'meta_input', ), ), - // Uses get_posts() with the $args if they are an array. - 'get_children' => array( 1 => array( 's', 'title' ) ), // Uses get_term_by( 'name' ). All of the other args are either integers or // accept slug-like values. 'get_bookmarks' => array( 1 => array( 'category_name' ) ), // Uses wp_unslash() on these. All of the other args are either integers or // accept slug-like values. 'get_pages' => array( 1 => array( 'meta_key', 'meta_value' ) ), - // Uses WP_Query::query(). - 'get_posts' => array( 1 => array( 's', 'title' ) ), // Uses wp_insert_attachment(). 'media_handle_sideload' => array( 4 => array( @@ -297,8 +309,6 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'meta_input', ), ), - // Uses WP_Query::query(). - 'query_posts' => array( 1 => array( 's', 'title' ) ), // Uses wp_unslash() on some of these. All of the other args are either // integers, slugs, or dates. 'wp_allow_comment' => array( @@ -311,12 +321,9 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'comment_agent', ), ), - // Uses get_posts(). - 'wp_get_nav_menu_items' => array( 2 => array( 's', 'title' ) ), - // Uses get_children(). + // Uses get_children(), but revisions don't support meta and tax info by + // default, so this is partly slashed for now, not mixed. 'wp_get_post_revisions' => array( 2 => array( 's', 'title' ) ), - // Uses get_posts(). - 'wp_get_recent_posts' => array( 1 => array( 's', 'title' ) ), // Uses wp_insert_post(). 'wp_insert_attachment' => array( 1 => array( @@ -412,16 +419,6 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { ), // Uses WP_Query::__construct(). 'WP_Customize_Nav_Menus::search_available_items_query' => array( 1 => array( 's' ) ), - // Uses WP_Query::query(). - 'WP_Query::__construct' => array( 1 => array( 's', 'title' ) ), - // WP_Query::get_posts() uses stripslashes() on the 'title', and - // WP_Query::parse_search() uses stripslashes() on 's'. All other args are - // either integers, booleans, or slug-like strings. - 'WP_Query::parse_query' => array( 1 => array( 's', 'title' ) ), - // WP_Query::get_posts() uses stripslashes() on the 'title', and - // WP_Query::parse_search() uses stripslashes() on 's'. All other args are - // either integers, booleans, or slug-like strings. - 'WP_Query::query' => array( 1 => array( 's', 'title' ) ), // Uses wp_insert_attachment(). 'WP_Site_Icon::insert_attachment' => array( 1 => array( @@ -458,7 +455,42 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { * @var array[] */ public static $mixedSlashedFunctionArgs = array( - // Uses get_bookmarks(). When 'categorize' is true, passing 'categroy_name' + // Uses get_posts() with the $args if they are an array. + 'get_children' => array( + 1 => array( + 'slashed' => array( 's', 'title' ), + 'unslashed' => array( 'meta_key', 'meta_query', 'meta_value', 'tax_query' ), + ), + ), + // Uses WP_Query::query(). + 'get_posts' => array( + 1 => array( + 'slashed' => array( 's', 'title' ), + 'unslashed' => array( 'meta_key', 'meta_query', 'meta_value', 'tax_query' ), + ), + ), + // Uses WP_Query::query(). + 'query_posts' => array( + 1 => array( + 'slashed' => array( 's', 'title' ), + 'unslashed' => array( 'meta_key', 'meta_query', 'meta_value', 'tax_query' ), + ), + ), + // Uses get_posts(). + 'wp_get_nav_menu_items' => array( + 2 => array( + 'slashed' => array( 's', 'title' ), + 'unslashed' => array( 'meta_key', 'meta_query', 'meta_value', 'tax_query' ), + ), + ), + // Uses get_posts(). + 'wp_get_recent_posts' => array( + 1 => array( + 'slashed' => array( 's', 'title' ), + 'unslashed' => array( 'meta_key', 'meta_query', 'meta_value', 'tax_query' ), + ), + ), + // Uses get_bookmarks(). When 'categorize' is true, passing 'category_name' // doesn't make sense, so this only applies when 'categorize' false. In fact, // when 'categorize' is true, the 'category_name' is passed to get_terms() as // 'name__like', which is expected unslashed. So it is only expected slashed @@ -528,6 +560,31 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { 'unslashed' => array( 'user_pass' ), ), ), + // Uses WP_Query::query(). + 'WP_Query::__construct' => array( + 1 => array( + 'slashed' => array( 's', 'title' ), + 'unslashed' => array( 'meta_key', 'meta_query', 'meta_value', 'tax_query' ), + ), + ), + // WP_Query::get_posts() uses stripslashes() on the 'title', and + // WP_Query::parse_search() uses stripslashes() on 's'. All other args are + // either integers, booleans, or slug-like strings. + 'WP_Query::parse_query' => array( + 1 => array( + 'slashed' => array( 's', 'title' ), + 'unslashed' => array( 'meta_key', 'meta_query', 'meta_value', 'tax_query' ), + ), + ), + // WP_Query::get_posts() uses stripslashes() on the 'title', and + // WP_Query::parse_search() uses stripslashes() on 's'. All other args are + // either integers, booleans, or slug-like strings. + 'WP_Query::query' => array( + 1 => array( + 'slashed' => array( 's', 'title' ), + 'unslashed' => array( 'meta_key', 'meta_query', 'meta_value', 'tax_query' ), + ), + ), ); /** From 3eed1f1aae3c656663b81ff87eed07c3bb1715bd Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Fri, 3 Nov 2017 17:06:31 -0400 Subject: [PATCH 19/21] Use a different error code when inside slashed function definitions --- WordPress/Sniffs/WP/ExpectedSlashedSniff.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php index 62350855e7..443e6bde5e 100644 --- a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php +++ b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php @@ -1165,7 +1165,7 @@ protected function slashing_check_loop( $start, $end, $errors ) { protected function addError( $error, $stackPtr, $code, $data ) { if ( $this->is_inside_slashed_function_definition( $stackPtr ) ) { - $this->phpcsFile->addWarning( $error, $stackPtr, $code, $data ); + $this->phpcsFile->addWarning( $error, $stackPtr, $code . 'Internal', $data ); } else { $this->phpcsFile->addError( $error, $stackPtr, $code, $data ); } From fa5d2c2e503ce83d7080ee291d9f0d9436c5747b Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Sat, 4 Nov 2017 10:36:08 -0400 Subject: [PATCH 20/21] Only warn about strings that contain slashes (not just quotes) --- WordPress/Sniffs/WP/ExpectedSlashedSniff.php | 22 +++++++++---------- .../Tests/WP/ExpectedSlashedUnitTest.inc | 14 ++++++------ .../Tests/WP/ExpectedSlashedUnitTest.php | 1 - 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php index 443e6bde5e..31c6e73112 100644 --- a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php +++ b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php @@ -891,9 +891,9 @@ protected function process_mixed_function_args( $parameters, $function_name ) { 'data' => array( $function_name, $key_name ), ); - $errors['slashable_string'] = array( + $errors['slashed_string'] = array( 'message' => '%s() expects the value of %s to be slashed with wp_slash(); %s found.', - 'code' => 'KeyStringMayNeedSlashing', + 'code' => 'KeyStringMayNeedDoubleSlashing', 'data' => array( $function_name, $key_name ) ); @@ -971,9 +971,9 @@ protected function process_expected_slashed_args( $parameters, $function_name ) 'code' => 'MissingSlashing', 'data' => array( $function_name, $name ) ), - 'slashable_string' => array( + 'slashed_string' => array( 'message' => '%s() expects the value of the $%s arg to be slashed with wp_slash(); %s found.', - 'code' => 'StringMayNeedSlashing', + 'code' => 'StringMayNeedDoubleSlashing', 'data' => array( $function_name, $name ) ) ) @@ -1006,8 +1006,8 @@ protected function process_expected_slashed_args( $parameters, $function_name ) * @type string $code The error code. * @type array $data The error data. The found content will be added. * } - * @type array $slashable_string { - * The warning to give when a slashable string is found. + * @type array $slashed_string { + * The warning to give when a slashed string is found. * * @type string $message The error message. * @type string $code The error code. @@ -1071,13 +1071,13 @@ protected function slashing_check_loop( $start, $end, $errors ) { if ( T_CONSTANT_ENCAPSED_STRING === $tokens[ $i ]['code'] ) { - if ( preg_match( '~^["\'].*["\'\\\\].*["\']$~', $content ) ) { - if ( isset( $errors['slashable_string'] ) ) { + if ( preg_match( '/^(["\']).*\\\\(?!\1).*\1$/', $content ) ) { + if ( isset( $errors['slashed_string'] ) ) { $this->phpcsFile->addWarning( - $errors['slashable_string']['message'], + $errors['slashed_string']['message'], $i, - $errors['slashable_string']['code'], - array_merge( $errors['slashable_string']['data'], array( $content ) ) + $errors['slashed_string']['code'], + array_merge( $errors['slashed_string']['data'], array( $content ) ) ); } } diff --git a/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc b/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc index da8d50d5f5..b17c203adb 100644 --- a/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc +++ b/WordPress/Tests/WP/ExpectedSlashedUnitTest.inc @@ -100,22 +100,22 @@ get_bookmarks(array("category" => $cat->term_id)); // OK. // String passed. add_post_meta( $post_id, 'key', 'value' ); // OK. -add_post_meta( $post_id, 'key', 'I said, "Wow!"' ); // Warning. -add_post_meta( $post_id, 'key', 'That\'s nice.' ); // Warning. +add_post_meta( $post_id, 'key', 'I said, \"Wow!\"' ); // Warning. +add_post_meta( $post_id, 'key', 'That\'s nice.' ); // OK. add_post_meta( $post_id, 'key', 'Slashes (\\).' ); // Warning. wp_update_term( $term_id, 'tag', array( 'name' => 'value' ) ); // OK. -wp_update_term( $term_id, 'tag', array( 'name' => 'I said, "Wow!"' ) ); // Warning. +wp_update_term( $term_id, 'tag', array( 'name' => 'slash\\s' ) ); // Warning. wp_list_pages( array( 'meta_value' => 'value' ) ); // OK. -wp_list_pages( array( 'meta_value' => 'I said, "Wow!"' ) ); // Warning. +wp_list_pages( array( 'meta_value' => 'slash\\s' ) ); // Warning. // Mixed. -add_post_meta( $post_id, 'key', wp_slash( $value ) . $value . ' "' ); // 1 Error, 1 Warning. +add_post_meta( $post_id, 'key', wp_slash( $value ) . $value . ' \"' ); // 1 Error, 1 Warning. -wp_update_term( $term_id, 'tag', array( 'name' => wp_slash( $value ) . $value . ' "' ) ); // 1 Error, 1 Warning. +wp_update_term( $term_id, 'tag', array( 'name' => wp_slash( $value ) . $value . ' \"' ) ); // 1 Error, 1 Warning. -wp_list_pages( array( 'meta_value' => wp_slash( $value ) . $value . ' "' ) ); // 1 Error, 1 Warning. +wp_list_pages( array( 'meta_value' => wp_slash( $value ) . $value . ' \"' ) ); // 1 Error, 1 Warning. // More slashed superglobals. wp_update_term( $term_id, 'tag', array( 'name' => $_POST['something'] ) ); // OK. diff --git a/WordPress/Tests/WP/ExpectedSlashedUnitTest.php b/WordPress/Tests/WP/ExpectedSlashedUnitTest.php index 1cd96740d2..6a7a4f4888 100644 --- a/WordPress/Tests/WP/ExpectedSlashedUnitTest.php +++ b/WordPress/Tests/WP/ExpectedSlashedUnitTest.php @@ -59,7 +59,6 @@ public function getWarningList() { 66 => 2, 70 => 1, 103 => 1, - 104 => 1, 105 => 1, 108 => 1, 111 => 1, From f321bed4c826bf708b7963f27f0a5a7f0b618254 Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Sat, 4 Nov 2017 11:08:19 -0400 Subject: [PATCH 21/21] Fix PHPCS errors --- WordPress/Sniffs/WP/ExpectedSlashedSniff.php | 567 +++++++++++++----- .../Tests/WP/ExpectedSlashedUnitTest.php | 8 + 2 files changed, 418 insertions(+), 157 deletions(-) diff --git a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php index 31c6e73112..e1f20ac060 100644 --- a/WordPress/Sniffs/WP/ExpectedSlashedSniff.php +++ b/WordPress/Sniffs/WP/ExpectedSlashedSniff.php @@ -44,11 +44,11 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { * @var int[] */ protected $slashed_tokens = array( - T_LNUMBER => true, - T_MINUS => true, - T_TRUE => true, - T_FALSE => true, - T_NULL => true, + T_LNUMBER => true, + T_MINUS => true, + T_TRUE => true, + T_FALSE => true, + T_NULL => true, ); /** @@ -60,172 +60,353 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { */ public static $expectedSlashedFunctionArgs = array( // Uses add_metadata(). - 'add_comment_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + 'add_comment_meta' => array( + 2 => 'meta_key', + 3 => 'meta_value', + ), // Uses wp_unslash(). - 'add_metadata' => array( 3 => 'meta_key', 4 => 'meta_value' ), + 'add_metadata' => array( + 3 => 'meta_key', + 4 => 'meta_value', + ), // Uses wp_unslash(). - 'add_ping' => array( 2 => 'uri' ), + 'add_ping' => array( + 2 => 'uri', + ), // Uses add_metadata(). - 'add_post_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + 'add_post_meta' => array( + 2 => 'meta_key', + 3 => 'meta_value', + ), // Uses add_metadata(). - 'add_term_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + 'add_term_meta' => array( + 2 => 'meta_key', + 3 => 'meta_value', + ), // Uses add_metadata(). - 'add_user_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + 'add_user_meta' => array( + 2 => 'meta_key', + 3 => 'meta_value', + ), // Uses term_exists(). - 'category_exists' => array( 1 => 'cat_name' ), + 'category_exists' => array( + 1 => 'cat_name', + ), // Uses wp_unslash(). - 'check_comment' => array( 1 => 'author', 2 => 'email' ), + 'check_comment' => array( + 1 => 'author', + 2 => 'email', + ), // Uses stripslashes(). - 'comment_exists' => array( 1 => 'comment_author', 2 => 'comment_date' ), + 'comment_exists' => array( + 1 => 'comment_author', + 2 => 'comment_date', + ), // Uses delete_metadata(). - 'delete_comment_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + 'delete_comment_meta' => array( + 2 => 'meta_key', + 3 => 'meta_value', + ), // Uses wp_unslash(). - 'delete_metadata' => array( 3 => 'meta_key', 4 => 'meta_value' ), + 'delete_metadata' => array( + 3 => 'meta_key', + 4 => 'meta_value', + ), // Uses delete_metadata(). - 'delete_post_meta_by_key' => array( 1 => 'post_meta_key' ), + 'delete_post_meta_by_key' => array( + 1 => 'post_meta_key', + ), // Uses delete_metadata(). - 'delete_post_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + 'delete_post_meta' => array( + 2 => 'meta_key', + 3 => 'meta_value', + ), // Uses delete_metadata(). - 'delete_term_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + 'delete_term_meta' => array( + 2 => 'meta_key', + 3 => 'meta_value', + ), // Uses delete_metadata(). - 'delete_user_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + 'delete_user_meta' => array( + 2 => 'meta_key', + 3 => 'meta_value', + ), // Uses delete_user_meta(). - 'delete_user_option' => array( 2 => 'option_name' ), + 'delete_user_option' => array( + 2 => 'option_name', + ), // Expects POST data. - 'edit_post' => array( 1 => 'post_data' ), + 'edit_post' => array( + 1 => 'post_data', + ), // Uses stripslashes(), but adds slashes back with addslashes() (which is // necessary escaping for the context the output is intended to be used in). - 'esc_js' => array( 1 => 'text' ), + 'esc_js' => array( + 1 => 'text', + ), // Uses get_term_by( 'name' ). - 'get_cat_ID' => array( 1 => 'category_name' ), + 'get_cat_ID' => array( + 1 => 'category_name', + ), // Uses get_term_by( 'name' ). - 'get_linkobjectsbyname' => array( 1 => 'cat_name' ), + 'get_linkobjectsbyname' => array( + 1 => 'cat_name', + ), // Uses get_term_by( 'name' ). - 'get_linksbyname' => array( 1 => 'cat_name' ), + 'get_linksbyname' => array( + 1 => 'cat_name', + ), // Uses get_linksbyname(). - 'get_linksbyname_withrating' => array( 1 => 'cat_name' ), + 'get_linksbyname_withrating' => array( + 1 => 'cat_name', + ), // Uses get_search_feed_link(). - 'get_search_comments_feed_link' => array( 1 => array( 'search_query' ) ), + 'get_search_comments_feed_link' => array( + 1 => array( 'search_query' ), + ), // Uses get_search_link(). - 'get_search_feed_link' => array( 1 => 'search_query' ), + 'get_search_feed_link' => array( + 1 => 'search_query', + ), // Uses stripslashes(). - 'get_search_link' => array( 1 => 'query' ), + 'get_search_link' => array( + 1 => 'query', + ), // Uses wp_unslash() when $field is 'name'. - 'get_term_by' => array( 2 => 'value' ), + 'get_term_by' => array( + 2 => 'value', + ), // Uses wp_unslash(). - 'install_blog' => array( 2 => 'blog_title' ), + 'install_blog' => array( + 2 => 'blog_title', + ), // Uses wp_get_nav_menu_object(). - 'is_nav_menu' => array( 1 => 'menu' ), + 'is_nav_menu' => array( + 1 => 'menu', + ), // Uses term_exists(). - 'is_term' => array( 1 => 'term' ), + 'is_term' => array( + 1 => 'term', + ), // Uses esc_js(). - 'js_escape' => array( 1 => 'text' ), + 'js_escape' => array( + 1 => 'text', + ), // Uses wp_unslash(). - 'post_exists' => array( 1 => 'title', 2 => 'content', 3 => 'date' ), + 'post_exists' => array( + 1 => 'title', + 2 => 'content', + 3 => 'date', + ), // Uses update_post_meta() when the $file isn't empty. - 'update_attached_file' => array( 2 => 'file' ), + 'update_attached_file' => array( + 2 => 'file', + ), // Uses update_metadata(). - 'update_comment_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + 'update_comment_meta' => array( + 2 => 'meta_key', + 3 => 'meta_value', + ), // Uses wp_unslash(). - 'update_metadata' => array( 3 => 'meta_key', 4 => 'meta_value' ), + 'update_metadata' => array( + 3 => 'meta_key', + 4 => 'meta_value', + ), // Uses wp_unslash(). - 'update_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + 'update_meta' => array( + 2 => 'meta_key', + 3 => 'meta_value', + ), // Uses update_metadata(). - 'update_post_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + 'update_post_meta' => array( + 2 => 'meta_key', + 3 => 'meta_value', + ), // Uses update_metadata(). - 'update_term_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + 'update_term_meta' => array( + 2 => 'meta_key', + 3 => 'meta_value', + ), // Uses update_metadata(). - 'update_user_meta' => array( 2 => 'meta_key', 3 => 'meta_value' ), + 'update_user_meta' => array( + 2 => 'meta_key', + 3 => 'meta_value', + ), // Uses stripslashes(). - 'update_usermeta' => array( 3 => 'meta_value' ), + 'update_usermeta' => array( + 3 => 'meta_value', + ), // Uses term_exists(). - 'tag_exists' => array( 1 => 'tag_name' ), + 'tag_exists' => array( + 1 => 'tag_name', + ), // Uses wp_unslash() when a string is passed; also accepts term ID. - 'term_exists' => array( 1 => 'term' ), + 'term_exists' => array( + 1 => 'term', + ), // Uses update_user_meta(). - 'update_user_option' => array( 2 => 'option_name', 3 => 'newvalue' ), + 'update_user_option' => array( + 2 => 'option_name', + 3 => 'newvalue', + ), // Uses wp_set_object_terms(). - 'wp_add_object_terms' => array( 2 => 'terms' ), + 'wp_add_object_terms' => array( + 2 => 'terms', + ), // Uses wp_set_post_tags(). - 'wp_add_post_tags' => array( 2 => 'tags' ), + 'wp_add_post_tags' => array( + 2 => 'tags', + ), // Uses category_exists() and wp_create_category(). - 'wp_create_categories' => array( 1 => 'categories' ), + 'wp_create_categories' => array( + 1 => 'categories', + ), // Uses category_exists() and wp_insert_category(). - 'wp_create_category' => array( 1 => 'cat_name' ), + 'wp_create_category' => array( + 1 => 'cat_name', + ), // Uses wp_update_nav_menu_object(). - 'wp_create_nav_menu' => array( 1 => 'menu_name' ), + 'wp_create_nav_menu' => array( + 1 => 'menu_name', + ), // Uses wp_unslash(). - 'wp_create_post_autosave' => array( 1 => 'post_data' ), + 'wp_create_post_autosave' => array( + 1 => 'post_data', + ), // Uses wp_create_term(). - 'wp_create_tag' => array( 1 => 'tag_name' ), + 'wp_create_tag' => array( + 1 => 'tag_name', + ), // Uses wp_insert_term() and term_exists(). - 'wp_create_term' => array( 1 => 'term_name' ), + 'wp_create_term' => array( + 1 => 'term_name', + ), // Uses wp_get_nav_menu_object(). - 'wp_delete_nav_menu' => array( 1 => 'menu' ), + 'wp_delete_nav_menu' => array( + 1 => 'menu', + ), // Just passed data through it, but is used by wp_new_comment(), // wp_update_comment(), etc. - 'wp_filter_comment' => array( 1 => 'commentarr' ), + 'wp_filter_comment' => array( + 1 => 'commentarr', + ), // Uses stripslashes(), but adds slashes back with addslashes(). - 'wp_filter_kses' => array( 1 => 'data' ), + 'wp_filter_kses' => array( + 1 => 'data', + ), // Uses stripslashes(), but adds slashes back with addslashes(). - 'wp_filter_nohtml_kses' => array( 1 => 'data' ), + 'wp_filter_nohtml_kses' => array( + 1 => 'data', + ), // Uses stripslashes(), but adds slashes back with addslashes(). - 'wp_filter_post_kses' => array( 1 => 'data' ), + 'wp_filter_post_kses' => array( + 1 => 'data', + ), // Uses wp_get_nav_menu_object(). - 'wp_get_nav_menu_items' => array( 1 => 'menu' ), + 'wp_get_nav_menu_items' => array( + 1 => 'menu', + ), // Uses get_term_by( 'name' ) if $menu is not a term object, ID, or slug. - 'wp_get_nav_menu_object' => array( 1 => 'menu' ), + 'wp_get_nav_menu_object' => array( + 1 => 'menu', + ), // Uses wp_unslash(). - 'wp_insert_comment' => array( 1 => 'commentdata' ), + 'wp_insert_comment' => array( + 1 => 'commentdata', + ), // Uses wp_unslash(). - 'wp_insert_link' => array( 1 => 'linkdata' ), + 'wp_insert_link' => array( + 1 => 'linkdata', + ), // Uses wp_unslash(). - 'wp_insert_term' => array( 1 => 'term' ), + 'wp_insert_term' => array( + 1 => 'term', + ), // Uses wp_insert_comment() and wp_allow_comment(). - 'wp_new_comment' => array( 1 => 'commentdata' ), + 'wp_new_comment' => array( + 1 => 'commentdata', + ), // Uses stripslashes(), but adds the slashes back with addslashes(). - 'wp_rel_nofollow' => array( 1 => 'text' ), + 'wp_rel_nofollow' => array( + 1 => 'text', + ), // Uses term_exists(). The docs for wp_remove_object_terms() says that it // takes only term slugs or IDs, but it is also possible to pass in the term // names, and in that case they must be slashed. - 'wp_remove_object_terms' => array( 2 => 'terms' ), + 'wp_remove_object_terms' => array( + 2 => 'terms', + ), // Uses term_exists(), and wp_insert_term() if the term doesn't exist and is // a string. The docs for wp_set_object_terms() says that it takes only term // slugs or IDs, but it is also possible to pass in the term names, and in // that case they must be slashed. - 'wp_set_object_terms' => array( 2 => 'terms' ), + 'wp_set_object_terms' => array( + 2 => 'terms', + ), // Uses wp_set_post_terms(). - 'wp_set_post_categories' => array( 2 => 'post_categories' ), + 'wp_set_post_categories' => array( + 2 => 'post_categories', + ), // Uses wp_set_post_categories(). - 'wp_set_post_cats' => array( 2 => 'post_categories' ), + 'wp_set_post_cats' => array( + 2 => 'post_categories', + ), // Uses wp_set_post_terms(). - 'wp_set_post_tags' => array( 2 => 'tags' ), + 'wp_set_post_tags' => array( + 2 => 'tags', + ), // Uses wp_set_object_terms(). - 'wp_set_post_terms' => array( 2 => 'terms' ), + 'wp_set_post_terms' => array( + 2 => 'terms', + ), // Uses update_post_meta(). - 'wp_update_attachment_metadata' => array( 2 => 'data' ), + 'wp_update_attachment_metadata' => array( + 2 => 'data', + ), // Uses wp_unslash(). - 'wp_update_comment' => array( 1 => 'commentarr' ), + 'wp_update_comment' => array( + 1 => 'commentarr', + ), // Uses wp_insert_link(). - 'wp_update_link' => array( 1 => 'linkdata' ), + 'wp_update_link' => array( + 1 => 'linkdata', + ), // Uses install_blog(). - 'wpmu_create_blog' => array( 3 => 'title' ), + 'wpmu_create_blog' => array( + 3 => 'title', + ), // Uses wp_unslash(). - 'wpmu_validate_blog_signup' => array( 2 => 'blog_title' ), + 'wpmu_validate_blog_signup' => array( + 2 => 'blog_title', + ), // Uses wp_unslash(). - 'wpmu_welcome_notification' => array( 4 => 'title' ), + 'wpmu_welcome_notification' => array( + 4 => 'title', + ), // Uses wp_unslash(). - 'WP_Press_This::side_load_images' => array( 2 => 'content' ), + 'WP_Press_This::side_load_images' => array( + 2 => 'content', + ), // Uses wp_unslash(). - 'WP_Customize_Setting::sanitize' => array( 1 => 'value' ), + 'WP_Customize_Setting::sanitize' => array( + 1 => 'value', + ), // Uses wp_unslash(). - 'WP_User_Search::__construct' => array( 1 => 'search_term' ), + 'WP_User_Search::__construct' => array( + 1 => 'search_term', + ), // Uses wp_insert_post() and wp_update_post(). - 'wp_xmlrpc_server::_insert_post' => array( 2 => 'content_struct' ), + 'wp_xmlrpc_server::_insert_post' => array( + 2 => 'content_struct', + ), // Uses wp_unslash() and add_term_meta(). - 'wp_xmlrpc_server::set_term_custom_fields' => array( 2 => 'fields' ), + 'wp_xmlrpc_server::set_term_custom_fields' => array( + 2 => 'fields', + ), // Uses wp_unslash() and add_post_meta(). - 'wp_xmlrpc_server::set_custom_fields' => array( 2 => 'fields' ), + 'wp_xmlrpc_server::set_custom_fields' => array( + 2 => 'fields', + ), ); /** @@ -251,7 +432,9 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { */ public static $partlySlashedFunctionArgs = array( // Uses WP_Query::__construct(), 'q' is passed as 's'. - '_wp_ajax_menu_quick_search' => array( 1 => array( 'q' ) ), + '_wp_ajax_menu_quick_search' => array( + 1 => array( 'q' ), + ), // Uses wp_update_post(). 'bulk_edit_posts' => array( 1 => array( @@ -271,10 +454,14 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { ), // Uses get_term_by( 'name' ). All of the other args are either integers or // accept slug-like values. - 'get_bookmarks' => array( 1 => array( 'category_name' ) ), + 'get_bookmarks' => array( + 1 => array( 'category_name' ), + ), // Uses wp_unslash() on these. All of the other args are either integers or // accept slug-like values. - 'get_pages' => array( 1 => array( 'meta_key', 'meta_value' ) ), + 'get_pages' => array( + 1 => array( 'meta_key', 'meta_value' ), + ), // Uses wp_insert_attachment(). 'media_handle_sideload' => array( 4 => array( @@ -323,7 +510,9 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { ), // Uses get_children(), but revisions don't support meta and tax info by // default, so this is partly slashed for now, not mixed. - 'wp_get_post_revisions' => array( 2 => array( 's', 'title' ) ), + 'wp_get_post_revisions' => array( + 2 => array( 's', 'title' ), + ), // Uses wp_insert_post(). 'wp_insert_attachment' => array( 1 => array( @@ -342,7 +531,9 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { ), ), // Uses wp_insert_term() and wp_update_term(). - 'wp_insert_category' => array( 1 => array( 'category_description', 'cat_name' ) ), + 'wp_insert_category' => array( + 1 => array( 'category_description', 'cat_name' ), + ), // Uses wp_unslash(). 'wp_insert_post' => array( 1 => array( @@ -363,23 +554,31 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { // Uses wp_unslash() on this. All of the other args are integers or slugs. // The 'name' arg is also expected slashed, but this is always overridden by // $term. - 'wp_insert_term' => array( 3 => array( 'description' ) ), + 'wp_insert_term' => array( + 3 => array( 'description' ), + ), // Uses wp_get_nav_menu_object() on this. - 'wp_nav_menu' => array( 1 => array( 'menu' ) ), + 'wp_nav_menu' => array( + 1 => array( 'menu' ), + ), // Uses wp_update_nav_menu_item(). 'wp_save_nav_menu_items' => array( - 2 => array( 'menu-item-description', 'menu-item-attr-title', 'menu-item-title' ) + 2 => array( 'menu-item-description', 'menu-item-attr-title', 'menu-item-title' ), ), // Uses wp_insert_category(). - 'wp_update_category' => array( 1 => array( 'category_description', 'cat_name' ) ), + 'wp_update_category' => array( + 1 => array( 'category_description', 'cat_name' ), + ), // Uses wp_insert_post() or wp_update_post(). All other values are slugs or // integers. 'wp_update_nav_menu_item' => array( - 3 => array( 'menu-item-description', 'menu-item-attr-title', 'menu-item-title' ) + 3 => array( 'menu-item-description', 'menu-item-attr-title', 'menu-item-title' ), ), // Uses get_term_by( 'name' ) with 'menu-name' and also passes the data to // wp_insert_term() if the menu doesn't exist, or else wp_update_term(). - 'wp_update_nav_menu_object' => array( 2 => array( 'description', 'menu-name' ) ), + 'wp_update_nav_menu_object' => array( + 2 => array( 'description', 'menu-name' ), + ), // Uses wp_insert_post(). If the $postarr is actually a post object and not // an array, then it should be unslashed instead. 'wp_update_post' => array( @@ -399,7 +598,9 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { ), ), // Uses wp_unslash() on these. All of the other args are integers or slugs. - 'wp_update_term' => array( 3 => array( 'description', 'name' ) ), + 'wp_update_term' => array( + 3 => array( 'description', 'name' ), + ), // Uses wp_insert_attachment(). 'Custom_Image_Header::insert_attachment' => array( 1 => array( @@ -418,7 +619,9 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { ), ), // Uses WP_Query::__construct(). - 'WP_Customize_Nav_Menus::search_available_items_query' => array( 1 => array( 's' ) ), + 'WP_Customize_Nav_Menus::search_available_items_query' => array( + 1 => array( 's' ), + ), // Uses wp_insert_attachment(). 'WP_Site_Icon::insert_attachment' => array( 1 => array( @@ -437,7 +640,9 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { ), ), // Uses WP_Query::query(). - '_WP_Editors::wp_link_query' => array( 1 => array( 's' ) ), + '_WP_Editors::wp_link_query' => array( + 1 => array( 's' ), + ), ); /** @@ -497,7 +702,7 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { // when 'categorize' is false. 'wp_list_bookmarks' => array( 1 => array( - 'slashed' => array( 'category_name' ), + 'slashed' => array( 'category_name' ), 'unslashed' => array( 'title_li', 'title_before', @@ -511,7 +716,7 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { // Uses get_pages(). 'wp_dropdown_pages' => array( 1 => array( - 'slashed' => array( 'meta_key', 'meta_value' ), + 'slashed' => array( 'meta_key', 'meta_value' ), 'unslashed' => array( 'selected', 'name', @@ -525,7 +730,7 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { // Uses get_pages(). 'wp_list_pages' => array( 1 => array( - 'slashed' => array( 'meta_key', 'meta_value' ), + 'slashed' => array( 'meta_key', 'meta_value' ), 'unslashed' => array( 'date_format', 'link_after', 'link_before', 'title_li' ), ), ), @@ -533,7 +738,7 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { // update_user_meta(). 'wp_insert_user' => array( 1 => array( - 'slashed' => array( + 'slashed' => array( 'description', 'display_name', 'first_name', @@ -548,7 +753,7 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { // Uses wp_insert_user(). 'wp_update_user' => array( 1 => array( - 'slashed' => array( + 'slashed' => array( 'description', 'display_name', 'first_name', @@ -598,45 +803,89 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { */ public $expectedSlashedFilterArgs = array( // Result passed through wp_unslash(). - 'add_ping' => array( 1 => 'new' ), + 'add_ping' => array( + 1 => 'new', + ), // Passed POSTed data. - 'attachment_fields_to_save' => array( 1 => 'post', 2 => 'attachment' ), + 'attachment_fields_to_save' => array( + 1 => 'post', + 2 => 'attachment', + ), // Result passed to wp_insert_attachment(). - 'attachment_thumbnail_args' => array( 1 => 'image_attachment' ), + 'attachment_thumbnail_args' => array( + 1 => 'image_attachment', + ), // Result passed to get_posts(). - 'dashboard_recent_drafts_query_args' => array( 1 => 'query_args' ), + 'dashboard_recent_drafts_query_args' => array( + 1 => 'query_args', + ), // Result passed through wp_unslash(). - 'pre_comment_author_email' => array( 1 => 'author_email_cookie' ), + 'pre_comment_author_email' => array( + 1 => 'author_email_cookie', + ), // Result passed through wp_unslash(). - 'pre_comment_author_name' => array( 1 => 'author_cookie' ), + 'pre_comment_author_name' => array( + 1 => 'author_cookie', + ), // Result passed through wp_unslash(). - 'pre_comment_author_url' => array( 1 => 'author_url_cookie' ), + 'pre_comment_author_url' => array( + 1 => 'author_url_cookie', + ), // Called in wp_filter_comment(). - 'pre_comment_content' => array( 1 => 'comment_content' ), + 'pre_comment_content' => array( + 1 => 'comment_content', + ), // Called in wp_filter_comment(). - 'pre_comment_user_agent' => array( 1 => 'comment_agent' ), + 'pre_comment_user_agent' => array( + 1 => 'comment_agent', + ), // Called in wp_insert_user(). - 'pre_user_description' => array( 1 => 'description' ), + 'pre_user_description' => array( + 1 => 'description', + ), // Called in wp_insert_user(). - 'pre_user_display_name' => array( 1 => 'display_name' ), + 'pre_user_display_name' => array( + 1 => 'display_name', + ), // Called in wp_insert_user(). - 'pre_user_email' => array( 1 => 'raw_user_email' ), + 'pre_user_email' => array( + 1 => 'raw_user_email', + ), // Called in wp_insert_user(). - 'pre_user_first_name' => array( 1 => 'first_name' ), + 'pre_user_first_name' => array( + 1 => 'first_name', + ), // Called in wp_insert_user(). - 'pre_user_last_name' => array( 1 => 'last_name' ), + 'pre_user_last_name' => array( + 1 => 'last_name', + ), // Called in wp_insert_user(). - 'pre_user_nickname' => array( 1 => 'nickname' ), + 'pre_user_nickname' => array( + 1 => 'nickname', + ), // Called in wp_insert_user(). - 'pre_user_url' => array( 1 => 'raw_user_url' ), + 'pre_user_url' => array( + 1 => 'raw_user_url', + ), // Passed POSTed data. - 'set-screen-option' => array( 1 => 'value', 2 => 'option', 3 => 'value' ), + 'set-screen-option' => array( + 1 => 'value', + 2 => 'option', + 3 => 'value', + ), // Result passed to wp_list_bookmarks(). - 'widget_links_args' => array( 1 => 'widget_links_args' ), + 'widget_links_args' => array( + 1 => 'widget_links_args', + ), // Result passed to wp_list_pages(). - 'widget_pages_args' => array( 1 => 'args' ), + 'widget_pages_args' => array( + 1 => 'args', + ), // Result passed to wp_update_post() or wp_insert_post(). - 'xmlrpc_wp_insert_post_data' => array( 1 => 'post_data', 2 => 'content_struct' ), + 'xmlrpc_wp_insert_post_data' => array( + 1 => 'post_data', + 2 => 'content_struct', + ), ); /** @@ -679,9 +928,9 @@ class ExpectedSlashedSniff extends AbstractFunctionParameterSniff { */ public function getGroups() { $this->target_functions = array_merge( - self::$expectedSlashedFunctionArgs - , self::$partlySlashedFunctionArgs - , self::$mixedSlashedFunctionArgs + self::$expectedSlashedFunctionArgs, + self::$partlySlashedFunctionArgs, + self::$mixedSlashedFunctionArgs ); return parent::getGroups(); @@ -747,13 +996,13 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p protected function process_mixed_function_args( $parameters, $function_name ) { $phpcsFile = $this->phpcsFile; - $tokens = $this->tokens; + $tokens = $this->tokens; if ( isset( self::$mixedSlashedFunctionArgs[ $function_name ] ) ) { - $args = self::$mixedSlashedFunctionArgs[ $function_name ]; + $args = self::$mixedSlashedFunctionArgs[ $function_name ]; $is_mixed = true; } else { - $args = self::$partlySlashedFunctionArgs[ $function_name ]; + $args = self::$partlySlashedFunctionArgs[ $function_name ]; $is_mixed = false; } @@ -775,10 +1024,10 @@ protected function process_mixed_function_args( $parameters, $function_name ) { ); if ( $is_mixed ) { - $slashed_keys = $keys['slashed']; + $slashed_keys = $keys['slashed']; $unslashed_keys = $keys['unslashed']; } else { - $slashed_keys = $keys; + $slashed_keys = $keys; $unslashed_keys = array(); } @@ -796,7 +1045,7 @@ protected function process_mixed_function_args( $parameters, $function_name ) { $function_name, implode( ', ', $slashed_keys ), implode( ', ', $unslashed_keys ), - $tokens[ $arg_start ]['content'] + $tokens[ $arg_start ]['content'], ) ); @@ -808,8 +1057,8 @@ protected function process_mixed_function_args( $parameters, $function_name ) { array( 'unslashed' => array( 'message' => '%s() expects the value of %s to be slashed with wp_slash(); %s found.', - 'code' => 'ExpectedPartlySlashed', - 'data' => array( $function_name, implode( ', ', $slashed_keys ) ), + 'code' => 'ExpectedPartlySlashed', + 'data' => array( $function_name, implode( ', ', $slashed_keys ) ), ), ) ); @@ -859,7 +1108,7 @@ protected function process_mixed_function_args( $parameters, $function_name ) { $function_name, implode( ', ', $slashed_keys ), implode( ', ', $unslashed_keys ), - $tokens[ $key_ptr ]['content'] + $tokens[ $key_ptr ]['content'], ); if ( $is_mixed ) { @@ -887,22 +1136,22 @@ protected function process_mixed_function_args( $parameters, $function_name ) { $errors['unslashed'] = array( 'message' => '%s() expects the value of %s to be slashed with wp_slash(); %s found.', - 'code' => 'ExpectedKeySlashed', - 'data' => array( $function_name, $key_name ), + 'code' => 'ExpectedKeySlashed', + 'data' => array( $function_name, $key_name ), ); $errors['slashed_string'] = array( 'message' => '%s() expects the value of %s to be slashed with wp_slash(); %s found.', - 'code' => 'KeyStringMayNeedDoubleSlashing', - 'data' => array( $function_name, $key_name ) + 'code' => 'KeyStringMayNeedDoubleSlashing', + 'data' => array( $function_name, $key_name ), ); } elseif ( in_array( $key_name, $unslashed_keys, true ) ) { $errors['slashed'] = array( 'message' => '%s() expects the value of %s to be unslashed; %s found.', - 'code' => 'ExpectedKeyUnslashed', - 'data' => array( $function_name, $key_name ), + 'code' => 'ExpectedKeyUnslashed', + 'data' => array( $function_name, $key_name ), ); } @@ -968,14 +1217,14 @@ protected function process_expected_slashed_args( $parameters, $function_name ) array( 'unslashed' => array( 'message' => '%s() expects the value of the $%s arg to be slashed with wp_slash(); %s found.', - 'code' => 'MissingSlashing', - 'data' => array( $function_name, $name ) + 'code' => 'MissingSlashing', + 'data' => array( $function_name, $name ), ), 'slashed_string' => array( 'message' => '%s() expects the value of the $%s arg to be slashed with wp_slash(); %s found.', - 'code' => 'StringMayNeedDoubleSlashing', - 'data' => array( $function_name, $name ) - ) + 'code' => 'StringMayNeedDoubleSlashing', + 'data' => array( $function_name, $name ), + ), ) ); } @@ -987,8 +1236,8 @@ protected function process_expected_slashed_args( $parameters, $function_name ) * * @since ${PROJECT_VERSION} * - * @param int $start The starting token position. - * @param int $end The ending token position. + * @param int $start The starting token position. + * @param int $end The ending token position. * @param array $errors { * The errors to give. * @@ -1021,6 +1270,11 @@ protected function slashing_check_loop( $start, $end, $errors ) { $in_cast = false; $watch = true; + $ignored = array( + T_DOUBLE_ARROW => true, + T_CLOSE_PARENTHESIS => true, + T_STRING_CONCAT => true, + ); for ( $i = $start; $i <= $end; $i++ ) { @@ -1041,13 +1295,13 @@ protected function slashing_check_loop( $start, $end, $errors ) { } // Ignore whitespaces and comments. - if ( in_array( $tokens[ $i ]['code'], Tokens::$emptyTokens ) ) { + if ( isset( Tokens::$emptyTokens[ $tokens[ $i ]['code'] ] ) ) { continue; } // Skip to the end of a function call if it has been casted to a safe value. if ( $in_cast && T_OPEN_PARENTHESIS === $tokens[ $i ]['code'] ) { - $i = $tokens[ $i ]['parenthesis_closer']; + $i = $tokens[ $i ]['parenthesis_closer']; $in_cast = false; continue; } @@ -1058,7 +1312,7 @@ protected function slashing_check_loop( $start, $end, $errors ) { continue; } - if ( in_array( $tokens[ $i ]['code'], array( T_DOUBLE_ARROW, T_CLOSE_PARENTHESIS, T_STRING_CONCAT ) ) ) { + if ( isset( $ignored[ $tokens[ $i ]['code'] ] ) ) { continue; } @@ -1124,7 +1378,6 @@ protected function slashing_check_loop( $start, $end, $errors ) { continue; } - } elseif ( T_VARIABLE === $tokens[ $i ]['code'] ) { if ( in_array( $content, $this->input_superglobals, true ) ) { @@ -1157,10 +1410,10 @@ protected function slashing_check_loop( $start, $end, $errors ) { * * @since ${PROJECT_VERSION} * - * @param string $error The error message. - * @param int $stackPtr The stack position where the error occurred. - * @param string $code A violation code unique to the sniff message. - * @param array $data Replacements for the error message. + * @param string $error The error message. + * @param int $stackPtr The stack position where the error occurred. + * @param string $code A violation code unique to the sniff message. + * @param array $data Replacements for the error message. */ protected function addError( $error, $stackPtr, $code, $data ) { diff --git a/WordPress/Tests/WP/ExpectedSlashedUnitTest.php b/WordPress/Tests/WP/ExpectedSlashedUnitTest.php index 6a7a4f4888..161f8ffcb4 100644 --- a/WordPress/Tests/WP/ExpectedSlashedUnitTest.php +++ b/WordPress/Tests/WP/ExpectedSlashedUnitTest.php @@ -21,7 +21,11 @@ class ExpectedSlashedUnitTest extends AbstractSniffUnitTest { /** + * Returns the lines where errors should occur. + * * @since + * + * @return array => */ public function getErrorList() { return array( @@ -49,7 +53,11 @@ public function getErrorList() { } /** + * Returns the lines where warnings should occur. + * * @since + * + * @return array => */ public function getWarningList() { return array(