From f3912b999befbd0341372cbaff2f30bc5a786531 Mon Sep 17 00:00:00 2001 From: Yannick Lefebvre Date: Sun, 2 Nov 2014 19:40:35 -0500 Subject: [PATCH 1/6] Added new class to check for forbidden functions only in template files Added new class that contains a list of file patterns for all template files, along with code that builds new patterns for all supported mime types. Use pattern array to identify all template files and check if they contain any of the forbidden functions. Addresses issue #23 --- .../TemplateForbiddenFunctionsCheck.php | 107 ++++++++++++++++++ vip-scanner/config-vip-scanner.php | 1 + 2 files changed, 108 insertions(+) create mode 100644 vip-scanner/checks/TemplateForbiddenFunctionsCheck.php diff --git a/vip-scanner/checks/TemplateForbiddenFunctionsCheck.php b/vip-scanner/checks/TemplateForbiddenFunctionsCheck.php new file mode 100644 index 0000000..f796e68 --- /dev/null +++ b/vip-scanner/checks/TemplateForbiddenFunctionsCheck.php @@ -0,0 +1,107 @@ +increment_check_count(); + + $theme_file_patterns = array( + '/([^-])404.php/', + '/([^-])archive(-([^\/]+))?.php/', + '/([^-])attachment.php/', + '/([^-])author(-(\d+|([^\/]+)))?.php/', + '/([^-])category(-(\d+|([^\/]+)))?.php/', + '/([^-])comments.php/', + '/([^-])(featured-)?content(-([^\/]+))?.php/', + '/([^-])date.php/', + '/([^-])footer.php/', + '/([^-])front-page.php/', + '/([^-])header.php/', + '/([^-])home.php/', + '/([^-])image.php/', + '/([^-])index.php/', + '/([^-])search.php/', + '/([^-])page(-(\d+|([^\/]+)))?.php/', + '/([^-])paged.php/', + '/([^-])sidebar(-([^\/]+))?.php/', + '/([^-])single(-(post|([^\/]+)))?.php/', + '/([^-])tag(-(\d+|([^\/]+)))?.php/', + '/([^-])taxonomy(-(\d+|([^\/]+)))?.php/', + ); + + $mime_types = get_allowed_mime_types(); + + foreach ( $mime_types as $extension => $mime_type ) { + $theme_file_patterns[] = '/' . str_replace( '/', '_', $mime_type ) . '.php/'; + + preg_match( '/(.+)\/(.+)/', $mime_type, $patterns ); + + if ( ! empty( $patterns ) ) { + for ( $index = 1; $index <= 2; $index ++ ) { + if ( isset( $patterns[ $index ] ) && ! empty( $patterns[ $index ] ) ) { + $new_pattern = '/(^-)' . str_replace( '/', '_', $patterns[ $index ] ) . '.php/'; + if ( ! in_array( $new_pattern, $theme_file_patterns ) ) { + $theme_file_patterns[] = $new_pattern; + } + } + } + } + } + + $checks = array( + 'auth_redirect', + 'maybe_redirect_404', + 'redirect_canonical', + 'redirect_post', + 'wp_old_slug_redirect', + 'wp_redirect', + 'wp_redirect_admin_locations', + 'wp_safe_redirect', + ); + + foreach ( $this->filter_files( $files, 'php' ) as $file_path => $file_content ) { + + // Match template files that can have any names + if ( preg_match( '/Template Name:/', $file_content, $file_matches ) ) { + if ( false == $this->check_file_contents( $checks, $file_path, $file_content ) ) { + $result = false; + } + } else { + // Test incoming file against all theme file name patterns + foreach ( $theme_file_patterns as $theme_file_pattern ) { + if ( preg_match( $theme_file_pattern, $file_path, $file_matches ) ) { + if ( false == $this->check_file_contents( $checks, $file_path, $file_content ) ) { + $result = false; + }; + } + } + } + } + + return $result; + } + + function check_file_contents( $checks, $file_path, $file_content ) { + $result = true; + + foreach ( $checks as $check ) { + if ( false !== strpos( $file_content, $check ) ) { + $lines = $this->grep_content( $check, $file_content ); + $this->add_error( + $check, + sprintf( 'Redirection functions should not be called from template files' ), + 'blocker', + $this->get_filename( $file_path ), + $lines + ); + $result = false; + } + } + + return $result; + } +} diff --git a/vip-scanner/config-vip-scanner.php b/vip-scanner/config-vip-scanner.php index 6f9763b..9399eeb 100644 --- a/vip-scanner/config-vip-scanner.php +++ b/vip-scanner/config-vip-scanner.php @@ -34,6 +34,7 @@ 'MasonryCheck', 'PostThumbnailsCheck', 'ScreenshotCheck', + 'TemplateForbiddenFunctionsCheck', 'ThemecolorsCheck', 'ThemeNameCheck', 'TitleCheck', From 8ce2bf600e5125d6b617a7f0634e49a5ba4da58a Mon Sep 17 00:00:00 2001 From: Yannick Lefebvre Date: Tue, 4 Nov 2014 20:50:45 -0500 Subject: [PATCH 2/6] Updated template filename patterns to better catch all files --- .../TemplateForbiddenFunctionsCheck.php | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/vip-scanner/checks/TemplateForbiddenFunctionsCheck.php b/vip-scanner/checks/TemplateForbiddenFunctionsCheck.php index f796e68..9c86308 100644 --- a/vip-scanner/checks/TemplateForbiddenFunctionsCheck.php +++ b/vip-scanner/checks/TemplateForbiddenFunctionsCheck.php @@ -10,27 +10,27 @@ function check( $files ) { $this->increment_check_count(); $theme_file_patterns = array( - '/([^-])404.php/', - '/([^-])archive(-([^\/]+))?.php/', - '/([^-])attachment.php/', - '/([^-])author(-(\d+|([^\/]+)))?.php/', - '/([^-])category(-(\d+|([^\/]+)))?.php/', - '/([^-])comments.php/', - '/([^-])(featured-)?content(-([^\/]+))?.php/', - '/([^-])date.php/', - '/([^-])footer.php/', - '/([^-])front-page.php/', - '/([^-])header.php/', - '/([^-])home.php/', - '/([^-])image.php/', - '/([^-])index.php/', - '/([^-])search.php/', - '/([^-])page(-(\d+|([^\/]+)))?.php/', - '/([^-])paged.php/', - '/([^-])sidebar(-([^\/]+))?.php/', - '/([^-])single(-(post|([^\/]+)))?.php/', - '/([^-])tag(-(\d+|([^\/]+)))?.php/', - '/([^-])taxonomy(-(\d+|([^\/]+)))?.php/', + '/(\/)?404.php/', + '/(\/)?archive(-([^\/]+))?.php/', + '/(\/)?attachment.php/', + '/(\/)?author(-(\d+|([^\/]+)))?.php/', + '/(\/)?category(-(\d+|([^\/]+)))?.php/', + '/(\/)?comments.php/', + '/(\/)?(featured-)?content(-([^\/]+))?.php/', + '/(\/)?date.php/', + '/(\/)?footer.php/', + '/(\/)?front-page.php/', + '/(\/)?header.php/', + '/(\/)?home.php/', + '/(\/)?image.php/', + '/(\/)?index.php/', + '/(\/)?search.php/', + '/(\/)?page(-(\d+|([^\/]+)))?.php/', + '/(\/)?paged.php/', + '/(\/)?sidebar(-([^\/]+))?.php/', + '/(\/)?single(-(post|([^\/]+)))?.php/', + '/(\/)?tag(-(\d+|([^\/]+)))?.php/', + '/(\/)?taxonomy(-(\d+|([^\/]+)))?.php/', ); $mime_types = get_allowed_mime_types(); From 401a722e701e7d828e8cdcbbe4e86e4421e2f6f5 Mon Sep 17 00:00:00 2001 From: Yannick Lefebvre Date: Tue, 4 Nov 2014 20:52:05 -0500 Subject: [PATCH 3/6] Added break in template file name pattern search once a match is found --- vip-scanner/checks/TemplateForbiddenFunctionsCheck.php | 1 + 1 file changed, 1 insertion(+) diff --git a/vip-scanner/checks/TemplateForbiddenFunctionsCheck.php b/vip-scanner/checks/TemplateForbiddenFunctionsCheck.php index 9c86308..7c06eb1 100644 --- a/vip-scanner/checks/TemplateForbiddenFunctionsCheck.php +++ b/vip-scanner/checks/TemplateForbiddenFunctionsCheck.php @@ -77,6 +77,7 @@ function check( $files ) { if ( false == $this->check_file_contents( $checks, $file_path, $file_content ) ) { $result = false; }; + break; } } } From 1f0e6e1d6f20fba3a8692f4ccee93fa6fd8f1588 Mon Sep 17 00:00:00 2001 From: Yannick Lefebvre Date: Tue, 4 Nov 2014 20:52:42 -0500 Subject: [PATCH 4/6] Simplified slug in add_error call to be the same for all matches found --- vip-scanner/checks/TemplateForbiddenFunctionsCheck.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vip-scanner/checks/TemplateForbiddenFunctionsCheck.php b/vip-scanner/checks/TemplateForbiddenFunctionsCheck.php index 7c06eb1..1676320 100644 --- a/vip-scanner/checks/TemplateForbiddenFunctionsCheck.php +++ b/vip-scanner/checks/TemplateForbiddenFunctionsCheck.php @@ -93,7 +93,7 @@ function check_file_contents( $checks, $file_path, $file_content ) { if ( false !== strpos( $file_content, $check ) ) { $lines = $this->grep_content( $check, $file_content ); $this->add_error( - $check, + 'template-forbidden-functions', sprintf( 'Redirection functions should not be called from template files' ), 'blocker', $this->get_filename( $file_path ), From 9e04337568053c0493c4a20914708f9ce002f64c Mon Sep 17 00:00:00 2001 From: Yannick Lefebvre Date: Tue, 4 Nov 2014 20:53:00 -0500 Subject: [PATCH 5/6] Adding unit tests for TemplateForbiddenFunctionsCheckTest class --- .../test-TemplateForbiddenFunctionsCheck.php | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 tests/checks/test-TemplateForbiddenFunctionsCheck.php diff --git a/tests/checks/test-TemplateForbiddenFunctionsCheck.php b/tests/checks/test-TemplateForbiddenFunctionsCheck.php new file mode 100644 index 0000000..02301e3 --- /dev/null +++ b/tests/checks/test-TemplateForbiddenFunctionsCheck.php @@ -0,0 +1,99 @@ +_TemplateForbiddenFunctionsCheck = new TemplateForbiddenFunctionsCheck(); + } + + public function testRedirectInHeader() { + $input = array( + 'php' => array( + 'header.php' => " + " + ) + ); + + $result = $this->_TemplateForbiddenFunctionsCheck->check( $input ); + + $errors = $this->_TemplateForbiddenFunctionsCheck->get_errors(); + + $error_slugs = wp_list_pluck( $errors, 'slug' ); + $this->assertContains( 'template-forbidden-functions', $error_slugs ); + $this->assertFalse( $result ); + } + + public function testRedirectInSpecificPageTemplate() { + $input = array( + 'php' => array( + 'page-about.php' => " + " + ) + ); + + $result = $this->_TemplateForbiddenFunctionsCheck->check( $input ); + + $errors = $this->_TemplateForbiddenFunctionsCheck->get_errors(); + + $error_slugs = wp_list_pluck( $errors, 'slug' ); + $this->assertContains( 'template-forbidden-functions', $error_slugs ); + $this->assertFalse( $result ); + } + + public function testSafeRedirectInPageTemplateWithinSubDirectory() { + $input = array( + 'php' => array( + 'page-templates/contributors.php' => " + " + ) + ); + + $result = $this->_TemplateForbiddenFunctionsCheck->check( $input ); + + $errors = $this->_TemplateForbiddenFunctionsCheck->get_errors(); + + $error_slugs = wp_list_pluck( $errors, 'slug' ); + $this->assertContains( 'template-forbidden-functions', $error_slugs ); + $this->assertFalse( $result ); + } + + public function testAllowedRedirectInFunctionsPHPFile() { + $input = array( + 'php' => array( + 'functions.php' => " + " + ) + ); + + $result = $this->_TemplateForbiddenFunctionsCheck->check( $input ); + + $errors = $this->_TemplateForbiddenFunctionsCheck->get_errors(); + + $error_slugs = wp_list_pluck( $errors, 'slug' ); + $this->assertNotContains( 'template-forbidden-functions', $error_slugs ); + $this->assertTrue( $result ); + } +} From 258a690fd2d21648e9d42d10c39dfe937f50885a Mon Sep 17 00:00:00 2001 From: Yannick Lefebvre Date: Wed, 5 Nov 2014 08:02:32 -0500 Subject: [PATCH 6/6] Updated add_error slug to generic component and parameter value This makes slugs unique and describes where they came from. --- tests/checks/test-TemplateForbiddenFunctionsCheck.php | 8 ++++---- vip-scanner/checks/TemplateForbiddenFunctionsCheck.php | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/checks/test-TemplateForbiddenFunctionsCheck.php b/tests/checks/test-TemplateForbiddenFunctionsCheck.php index 02301e3..3b26112 100644 --- a/tests/checks/test-TemplateForbiddenFunctionsCheck.php +++ b/tests/checks/test-TemplateForbiddenFunctionsCheck.php @@ -27,7 +27,7 @@ public function testRedirectInHeader() { $errors = $this->_TemplateForbiddenFunctionsCheck->get_errors(); $error_slugs = wp_list_pluck( $errors, 'slug' ); - $this->assertContains( 'template-forbidden-functions', $error_slugs ); + $this->assertContains( 'template-forbidden-functions-wp_redirect', $error_slugs ); $this->assertFalse( $result ); } @@ -48,7 +48,7 @@ public function testRedirectInSpecificPageTemplate() { $errors = $this->_TemplateForbiddenFunctionsCheck->get_errors(); $error_slugs = wp_list_pluck( $errors, 'slug' ); - $this->assertContains( 'template-forbidden-functions', $error_slugs ); + $this->assertContains( 'template-forbidden-functions-wp_redirect', $error_slugs ); $this->assertFalse( $result ); } @@ -72,7 +72,7 @@ public function testSafeRedirectInPageTemplateWithinSubDirectory() { $errors = $this->_TemplateForbiddenFunctionsCheck->get_errors(); $error_slugs = wp_list_pluck( $errors, 'slug' ); - $this->assertContains( 'template-forbidden-functions', $error_slugs ); + $this->assertContains( 'template-forbidden-functions-wp_safe_redirect', $error_slugs ); $this->assertFalse( $result ); } @@ -93,7 +93,7 @@ public function testAllowedRedirectInFunctionsPHPFile() { $errors = $this->_TemplateForbiddenFunctionsCheck->get_errors(); $error_slugs = wp_list_pluck( $errors, 'slug' ); - $this->assertNotContains( 'template-forbidden-functions', $error_slugs ); + $this->assertNotContains( 'template-forbidden-functions-wp_redirect', $error_slugs ); $this->assertTrue( $result ); } } diff --git a/vip-scanner/checks/TemplateForbiddenFunctionsCheck.php b/vip-scanner/checks/TemplateForbiddenFunctionsCheck.php index 1676320..3a5f228 100644 --- a/vip-scanner/checks/TemplateForbiddenFunctionsCheck.php +++ b/vip-scanner/checks/TemplateForbiddenFunctionsCheck.php @@ -93,7 +93,7 @@ function check_file_contents( $checks, $file_path, $file_content ) { if ( false !== strpos( $file_content, $check ) ) { $lines = $this->grep_content( $check, $file_content ); $this->add_error( - 'template-forbidden-functions', + 'template-forbidden-functions-' . $check, sprintf( 'Redirection functions should not be called from template files' ), 'blocker', $this->get_filename( $file_path ),