From 1520bba02f33fb7177f6a265e8cd53a6c467ff3a Mon Sep 17 00:00:00 2001 From: "Gudmundur D. Haraldsson" Date: Fri, 11 Mar 2016 16:28:13 +0000 Subject: [PATCH] Adding three checks for non-escaped localization function calls. The first checks if output of the __( ), _x( ), _n( ), _nx( ) functions is being printed directly, via echo( ), print( ) or vprintf( ) -- care is taken to ensure that both echo with and without brackets is considered, and same with print. These are blockers. The second checks if __( ), _x( ), _n( ), _nx( ) functions are being called (without printing). These are warnings. The third checks if _e( ) or _ex( ) are being called. These are blockers. These checks are all against localization functions which do not HTML-escape. Not escaping can be dangerous since HTML code could be injected via malicous translation strings. Blocking was considered prudent with direct printing, because there is no escaping involved, whereas warnings are issued when strings are possibly escaped later on in processing. --- tests/checks/test-VIPEscapingCheck.php | 101 ++++++++++++++++++++++++ tests/data/VIPEscapingTest.inc | 30 +++++++ vip-scanner/checks/VIPEscapingCheck.php | 75 ++++++++++++++++++ vip-scanner/config-vip-scanner.php | 1 + 4 files changed, 207 insertions(+) create mode 100644 tests/checks/test-VIPEscapingCheck.php create mode 100644 tests/data/VIPEscapingTest.inc create mode 100644 vip-scanner/checks/VIPEscapingCheck.php diff --git a/tests/checks/test-VIPEscapingCheck.php b/tests/checks/test-VIPEscapingCheck.php new file mode 100644 index 0000000..8655198 --- /dev/null +++ b/tests/checks/test-VIPEscapingCheck.php @@ -0,0 +1,101 @@ + 'functions-file', + 'level' => BaseScanner::LEVEL_BLOCKER, + 'description' => sprintf( + esc_html__( 'Printing output of non-escaping localization functions (i.e. %1$s) is potentially dangerous, as they do not escape HTML. An escaping function (e.g. %2$s) should be used rather.', 'vip-scanner' ), + '__( ), _x( ), _n( ), _nx( )', + 'esc_html__( ), esc_attr__( ), esc_html_x( ), esc_attr_x( )' + ), + 'file' => 'VIPEscapingTest.inc', + 'lines' => $lines++, + ); + } + + $lines++; + + + /* + * Now test for non-printing usage of __( ), _x( ), + * _n( ), and _nx( ). + * + * Note: If you find yourself adding 'ifs' within the + * for-loop, stop using a loop and write out each + * expected error individually. + */ + + for ($i = 0; $i < 4; $i++) { + $expected_errors[] = array( + 'slug' => 'functions-file', + 'level' => BaseScanner::LEVEL_WARNING, + 'description' => sprintf( + esc_html__( 'Usage of non-escaping localization functions (i.e. %1$s) is discouraged as they do not escape HTML. An escaping function (e.g. %2$s) should be used rather.', 'vip-scanner' ), + '__( ), _x( ), _n( ), _nx( )', + 'esc_html__( ), esc_attr__( ), esc_html_x( ), esc_attr_x( )' + ), + 'file' => 'VIPEscapingTest.inc', + 'lines' => $lines++, + ); + } + + + /* + * Now test for usage of _e( ), _ex( ). + * + * Note: If you find yourself adding 'ifs' within the + * for-loop, stop using a loop and write out each + * expected error individually. + */ + + for ($i = 0; $i < 2; $i++) { + $expected_errors[] = array( + 'slug' => 'functions-file', + 'level' => BaseScanner::LEVEL_BLOCKER, + 'description' => sprintf( + esc_html__( 'Usage of non-escaping localization functions (i.e. %1$s) is discouraged as they do not escape HTML. An escaping function (e.g. %2$s) should be used rather.', 'vip-scanner' ), + '_e( ), _ex( )', + 'esc_html_e( ), esc_attr_e( ), esc_html_x( ), esc_attr_x( ), esc_attr__( )' + ), + 'file' => 'VIPEscapingTest.inc', + 'lines' => $lines++, + ); + } + + + $actual_errors = $this->checkFile( 'VIPEscapingTest.inc' ); + $this->assertEqualErrors( $expected_errors, $actual_errors ); + } +} diff --git a/tests/data/VIPEscapingTest.inc b/tests/data/VIPEscapingTest.inc new file mode 100644 index 0000000..c7ab068 --- /dev/null +++ b/tests/data/VIPEscapingTest.inc @@ -0,0 +1,30 @@ + '/([\s]|[;]){1,}(echo|vprintf|print)([\s]|[\(])+(__|_x|_n|_nx)[\s]*?\(/', + 'level' => 'blocker', + 'message' => sprintf( + esc_html__( 'Printing output of non-escaping localization functions (i.e. %1$s) is potentially dangerous, as they do not escape HTML. An escaping function (e.g. %2$s) should be used rather.', 'vip-scanner' ), + '__( ), _x( ), _n( ), _nx( )', + 'esc_html__( ), esc_attr__( ), esc_html_x( ), esc_attr_x( )' + ), + ), + array( + /* + * Catch non-printing usage of __( ), etc + * These are warnings, because the results + * are not being immediately printed, and so + * could be escaped at a later point. + */ + 'pattern' => '/([\s]|[;]|[=]){1,}[a-zA-Z]{0}[\s]+(__|_x|_n|_nx)[\s]*?\(/', + 'level' => 'warning', + 'message' => sprintf( + esc_html__( 'Usage of non-escaping localization functions (i.e. %1$s) is discouraged as they do not escape HTML. An escaping function (e.g. %2$s) should be used rather.', 'vip-scanner' ), + '__( ), _x( ), _n( ), _nx( )', + 'esc_html__( ), esc_attr__( ), esc_html_x( ), esc_attr_x( )' + ), + ), + array( + /* + * Catch calls to _e( ), _ex( ) + * These print directly, without HTML-escaping, + * and so are blockers. + */ + 'pattern' => '/([\s]|[;]){1,}[a-zA-Z]{0}[\s]+(_e|_ex)[\s]*?\(/', + 'level' => 'blocker', + 'message' => sprintf( + esc_html__( 'Usage of non-escaping localization functions (i.e. %1$s) is discouraged as they do not escape HTML. An escaping function (e.g. %2$s) should be used rather.', 'vip-scanner' ), + '_e( ), _ex( )', + 'esc_html_e( ), esc_attr_e( ), esc_html_x( ), esc_attr_x( ), esc_attr__( )' + ), + ), + ); + + $result = true; + foreach ( $checks as $check ) { + $this->increment_check_count(); + foreach ( $this->filter_files( $files, 'php' ) as $path => $code ) { + $filename = $this->get_filename( $path ); + $errors = $this->preg_file2( $check['pattern'], $code ); + foreach ( (array) $errors as $line_number => $error ) { + $this->add_error( + 'functions-file', + $check['message'], + $check['level'], + $filename, + array( $line_number => $error ) + ); + $result = false; + } + } + } + return $result; + } +} diff --git a/vip-scanner/config-vip-scanner.php b/vip-scanner/config-vip-scanner.php index e9f688b..1aeb9ed 100644 --- a/vip-scanner/config-vip-scanner.php +++ b/vip-scanner/config-vip-scanner.php @@ -59,6 +59,7 @@ 'DeprecatedFunctionsCheck', 'DeprecatedParametersCheck', 'jQueryCheck', + 'VIPEscapingCheck', 'VIPWhitelistCheck', 'VIPRestrictedClassesCheck', 'VIPRestrictedPatternsCheck',