From d83ecd4a4a4d2f114b7ef7f47e81c31b2354e4a3 Mon Sep 17 00:00:00 2001 From: Stefan Kalscheuer Date: Wed, 1 Aug 2018 20:18:15 +0200 Subject: [PATCH 1/6] Refactored JS tracking to use WP AJAX The custom GET request intercepts normal page generation and might trigger other plugins' actions before Statify is loaded. It also provided an open door for lightweight malicious requests targeting the statistics. Using WP AJAX including Nonce verification reduces both problems. --- CHANGELOG.md | 3 ++ inc/class-statify-frontend.php | 66 +++++++++++++++++++++++++--------- inc/class-statify.php | 8 +++-- js/snippet.js | 20 ++++++----- statify.php | 1 + views/js-snippet.php | 21 ----------- 6 files changed, 70 insertions(+), 49 deletions(-) delete mode 100644 views/js-snippet.php diff --git a/CHANGELOG.md b/CHANGELOG.md index b533d1b..8e83f62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ # Changelog All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). +## Unreleased +* Refactored JavaScript tracking to use WP AJAX + ## 1.6.3 * Fix compatibility issue with some PHP implementations not populating `INPUT_SERVER` * Fix failing blacklist check for empty referrers diff --git a/inc/class-statify-frontend.php b/inc/class-statify-frontend.php index 46a3603..81667b1 100644 --- a/inc/class-statify-frontend.php +++ b/inc/class-statify-frontend.php @@ -22,20 +22,32 @@ class Statify_Frontend extends Statify { * Track the page view * * @since 0.1.0 - * @version 1.4.2 + * @since 1.7.0 $is_snippet parameter added. + * @version 1.7.0 * - * @return bool + * @param boolean $is_snippet Is tracking triggered via JS (default: false). + * + * @return boolean */ - public static function track_visit() { + public static function track_visit( $is_snippet = false ) { /* Init vars */ $use_snippet = self::$_options['snippet']; - $is_snippet = $use_snippet && get_query_var( 'statify_target' ); /* Set target & referrer */ - if ( $is_snippet ) { - $target = urldecode( get_query_var( 'statify_target' ) ); - $referrer = urldecode( get_query_var( 'statify_referrer' ) ); + if ( $use_snippet && $is_snippet ) { + $target = urldecode( + filter_var( + ( isset( $_REQUEST['statify_target'] ) ? wp_unslash( $_REQUEST['statify_target'] ) : '/' ), + FILTER_SANITIZE_URL + ) + ); + $referrer = urldecode( + filter_var( + ( isset( $_REQUEST['statify_referrer'] ) ? wp_unslash( $_REQUEST['statify_referrer'] ) : '' ), + FILTER_SANITIZE_URL + ) + ); } elseif ( ! $use_snippet ) { $target = filter_var( ( isset( $_SERVER['REQUEST_URI'] ) ? wp_unslash( $_SERVER['REQUEST_URI'] ) : '/' ), @@ -116,6 +128,20 @@ public static function track_visit() { return self::_jump_out( $is_snippet ); } + /** + * Track the page view via AJAX. + * + * @return void + */ + public static function track_visit_ajax() { + // Check AJAX referrer. + check_ajax_referer( 'statify_track' ); + // Only do something if snippet use is actually configured. + if ( self::$_options['snippet'] ) { + self::track_visit( true ); + } + } + /** * Find the position of the first occurrence of a substring in a string about a array. * @@ -298,23 +324,31 @@ public static function query_vars( $vars ) { */ public static function wp_footer() { - /* Skip by option */ + // Skip by option. if ( ! self::$_options['snippet'] ) { return; } - /* Skip by internal rules (#84) */ + // Skip by internal rules (#84). if ( self::_is_internal() ) { return; } - /* Load template */ - load_template( - wp_normalize_path( - sprintf( - '%s/views/js-snippet.php', - STATIFY_DIR - ) + wp_enqueue_script( + 'statify-js', + plugins_url( 'js/snippet.js', STATIFY_FILE ), + array(), + STATIFY_VERSION, + true + ); + + // Add endpoint to script. + wp_localize_script( + 'statify-js', + 'statify_ajax', + array( + 'url' => admin_url( 'admin-ajax.php' ), + 'nonce' => wp_create_nonce( 'statify_track' ), ) ); } diff --git a/inc/class-statify.php b/inc/class-statify.php index a04a8c2..28b8436 100644 --- a/inc/class-statify.php +++ b/inc/class-statify.php @@ -44,8 +44,8 @@ public static function instance() { * @version 2017-01-10 */ public function __construct() { - // Skip me! - if ( ( defined( 'DOING_AUTOSAVE' ) && DOING_AUTOSAVE ) || ( defined( 'DOING_AJAX' ) && DOING_AJAX ) ) { + // Nothing to do on autosave. + if ( defined( 'DOING_AUTOSAVE' ) && DOING_AUTOSAVE ) { return; } @@ -64,7 +64,9 @@ public function __construct() { ) ); - if ( defined( 'XMLRPC_REQUEST' ) && XMLRPC_REQUEST ) { // XMLRPC. + if ( defined( 'DOING_AJAX' ) && DOING_AJAX ) { + add_action( 'wp_ajax_nopriv_statify_track', array( 'Statify_Frontend', 'track_visit_ajax' ) ); + } elseif ( defined( 'XMLRPC_REQUEST' ) && XMLRPC_REQUEST ) { // XMLRPC. add_filter( 'xmlrpc_methods', array( 'Statify_XMLRPC', 'xmlrpc_methods' ) ); } elseif ( defined( 'DOING_CRON' ) && DOING_CRON ) { // Cron. add_action( 'statify_cleanup', array( 'Statify_Cron', 'cleanup_data' ) ); diff --git a/js/snippet.js b/js/snippet.js index f7f9d99..9fd3208 100644 --- a/js/snippet.js +++ b/js/snippet.js @@ -1,11 +1,13 @@ try { - const statifyReq = new XMLHttpRequest(); - statifyReq.open( - 'GET', - document.getElementById('statify-js-snippet').getAttribute('data-home-url') - + '?statify_referrer=' + encodeURIComponent(document.referrer) - + '&statify_target=' + encodeURIComponent(location.pathname + location.search) - ); - statifyReq.send(null); -} catch (e) { + jQuery.ajax( { + type: 'POST', + url : statify_ajax.url, + data: { + _ajax_nonce : statify_ajax.nonce, + action : 'statify_track', + statify_referrer: encodeURIComponent( document.referrer ), + statify_target : encodeURIComponent( location.pathname + location.search ) + } + } ); +} catch ( e ) { } diff --git a/statify.php b/statify.php index bde262c..d881374 100644 --- a/statify.php +++ b/statify.php @@ -20,6 +20,7 @@ define( 'STATIFY_FILE', __FILE__ ); define( 'STATIFY_DIR', dirname( __FILE__ ) ); define( 'STATIFY_BASE', plugin_basename( __FILE__ ) ); +define( 'STATIFY_VERSION', '1.6.3' ); /* Hooks */ diff --git a/views/js-snippet.php b/views/js-snippet.php deleted file mode 100644 index f97821b..0000000 --- a/views/js-snippet.php +++ /dev/null @@ -1,21 +0,0 @@ - - - - - - - From 52f9fcd2e988e272035cce522f3d6a999143b43f Mon Sep 17 00:00:00 2001 From: Stefan Kalscheuer Date: Thu, 2 Aug 2018 20:57:18 +0200 Subject: [PATCH 2/6] Move sanitization of target and referrer out of if-else block --- inc/class-statify-frontend.php | 50 +++++++++++++--------------------- 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/inc/class-statify-frontend.php b/inc/class-statify-frontend.php index 81667b1..2af4866 100644 --- a/inc/class-statify-frontend.php +++ b/inc/class-statify-frontend.php @@ -31,45 +31,33 @@ class Statify_Frontend extends Statify { */ public static function track_visit( $is_snippet = false ) { - /* Init vars */ + // Check of JS snippet is configured. $use_snippet = self::$_options['snippet']; - /* Set target & referrer */ + // Set target & referrer. if ( $use_snippet && $is_snippet ) { - $target = urldecode( - filter_var( - ( isset( $_REQUEST['statify_target'] ) ? wp_unslash( $_REQUEST['statify_target'] ) : '/' ), - FILTER_SANITIZE_URL - ) - ); - $referrer = urldecode( - filter_var( - ( isset( $_REQUEST['statify_referrer'] ) ? wp_unslash( $_REQUEST['statify_referrer'] ) : '' ), - FILTER_SANITIZE_URL - ) - ); + $target = urldecode( isset( $_REQUEST['statify_target'] ) ? wp_unslash( $_REQUEST['statify_target'] ) : '/' ); + $referrer = urldecode( isset( $_REQUEST['statify_referrer'] ) ? wp_unslash( $_REQUEST['statify_referrer'] ) : '' ); } elseif ( ! $use_snippet ) { - $target = filter_var( - ( isset( $_SERVER['REQUEST_URI'] ) ? wp_unslash( $_SERVER['REQUEST_URI'] ) : '/' ), - FILTER_SANITIZE_URL - ); - if ( is_null( $target ) || false === $target ) { - $target = '/'; - } else { - $target = wp_unslash( $target ); - } - - $referrer = filter_var( - ( isset( $_SERVER['HTTP_REFERER'] ) ? wp_unslash( $_SERVER['HTTP_REFERER'] ) : '' ), - FILTER_SANITIZE_URL - ); - if ( is_null( $referrer ) || false === $referrer ) { - $referrer = ''; - } + $target = isset( $_SERVER['REQUEST_URI'] ) ? wp_unslash( $_SERVER['REQUEST_URI'] ) : '/'; + $referrer = isset( $_SERVER['HTTP_REFERER'] ) ? wp_unslash( $_SERVER['HTTP_REFERER'] ) : ''; } else { return false; } + // Sanitize. + $target = filter_var( $target, FILTER_SANITIZE_URL ); + if ( is_null( $target ) || false === $target ) { + $target = '/'; + } else { + $target = wp_unslash( $target ); + } + + $referrer = filter_var( $referrer, FILTER_SANITIZE_URL ); + if ( is_null( $referrer ) || false === $referrer ) { + $referrer = ''; + } + /* Invalid target? */ if ( empty( $target ) || ! wp_validate_redirect( $target, false ) ) { return self::_jump_out( $is_snippet ); From 74ca426facdc002f0284df23eacf12a0f0690cbe Mon Sep 17 00:00:00 2001 From: Stefan Kalscheuer Date: Tue, 16 Oct 2018 19:28:40 +0200 Subject: [PATCH 3/6] Reset AJAX request from jQuery back to vanilla JS with XmlHttpRequest --- js/snippet.js | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/js/snippet.js b/js/snippet.js index 9fd3208..9eaff2b 100644 --- a/js/snippet.js +++ b/js/snippet.js @@ -1,13 +1,12 @@ try { - jQuery.ajax( { - type: 'POST', - url : statify_ajax.url, - data: { - _ajax_nonce : statify_ajax.nonce, - action : 'statify_track', - statify_referrer: encodeURIComponent( document.referrer ), - statify_target : encodeURIComponent( location.pathname + location.search ) - } - } ); + var statifyReq = new XMLHttpRequest(); + statifyReq.open( 'POST', statify_ajax.url, true ); + statifyReq.setRequestHeader( 'Content-Type', 'application/x-www-form-urlencoded;' ); + statifyReq.send( + '_ajax_nonce=' + statify_ajax.nonce + + '&action=statify_track' + + '&statify_referrer=' + encodeURIComponent( document.referrer ) + + '&statify_target=' + encodeURIComponent( location.pathname + location.search ) + ); } catch ( e ) { } From 5ab18231e61114d0f9b97c46734d55560d17e9ba Mon Sep 17 00:00:00 2001 From: Stefan Kalscheuer Date: Thu, 4 Apr 2019 20:02:52 +0200 Subject: [PATCH 4/6] Remove superflous sanitization in pre-sanitized var in target filter --- inc/class-statify-frontend.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/inc/class-statify-frontend.php b/inc/class-statify-frontend.php index e68f6c1..4e3c147 100644 --- a/inc/class-statify-frontend.php +++ b/inc/class-statify-frontend.php @@ -49,8 +49,6 @@ public static function track_visit( $is_snippet = false ) { $target = filter_var( $target, FILTER_SANITIZE_URL ); if ( is_null( $target ) || false === $target ) { $target = '/'; - } else { - $target = wp_unslash( $target ); } $referrer = filter_var( $referrer, FILTER_SANITIZE_URL ); From 7705dff94a0db07572547b542992fc885e572608 Mon Sep 17 00:00:00 2001 From: Stefan Kalscheuer Date: Mon, 13 Apr 2020 14:45:40 +0200 Subject: [PATCH 5/6] rework target and referrer sanitization again --- inc/class-statify-frontend.php | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/inc/class-statify-frontend.php b/inc/class-statify-frontend.php index 4e3c147..bd50eb0 100644 --- a/inc/class-statify-frontend.php +++ b/inc/class-statify-frontend.php @@ -35,23 +35,30 @@ public static function track_visit( $is_snippet = false ) { $use_snippet = self::$_options['snippet']; // Set target & referrer. + $target = null; + $referrer = null; if ( $use_snippet && $is_snippet ) { - $target = urldecode( isset( $_REQUEST['statify_target'] ) ? wp_unslash( $_REQUEST['statify_target'] ) : '/' ); - $referrer = urldecode( isset( $_REQUEST['statify_referrer'] ) ? wp_unslash( $_REQUEST['statify_referrer'] ) : '' ); + if ( isset( $_REQUEST['statify_target'] ) ) { + $target = filter_var( wp_unslash( $_REQUEST['statify_target'] ), FILTER_SANITIZE_URL ); + } + if ( isset( $_REQUEST['statify_referrer'] ) ) { + $referrer = filter_var( wp_unslash( $_REQUEST['statify_referrer'] ), FILTER_SANITIZE_URL ); + } } elseif ( ! $use_snippet ) { - $target = isset( $_SERVER['REQUEST_URI'] ) ? wp_unslash( $_SERVER['REQUEST_URI'] ) : '/'; - $referrer = isset( $_SERVER['HTTP_REFERER'] ) ? wp_unslash( $_SERVER['HTTP_REFERER'] ) : ''; + if ( isset( $_SERVER['REQUEST_URI'] ) ) { + $target = filter_var( wp_unslash( $_SERVER['REQUEST_URI'] ), FILTER_SANITIZE_URL ); + } + if ( isset( $_SERVER['HTTP_REFERER'] ) ) { + $referrer = filter_var( wp_unslash( $_SERVER['HTTP_REFERER'] ), FILTER_SANITIZE_URL ); + } } else { return false; } - // Sanitize. - $target = filter_var( $target, FILTER_SANITIZE_URL ); + // Fallbacks for uninitialized or omitted target and referrer values. if ( is_null( $target ) || false === $target ) { $target = '/'; } - - $referrer = filter_var( $referrer, FILTER_SANITIZE_URL ); if ( is_null( $referrer ) || false === $referrer ) { $referrer = ''; } From ae543f928c329f420b598163b4f5219ee7873abe Mon Sep 17 00:00:00 2001 From: Patrick Robrecht Date: Mon, 13 Apr 2020 20:36:02 +0200 Subject: [PATCH 6/6] Use minifed version of the snippet, set version to 1.7.0 (consistent with the PHPDoc) --- inc/class-statify-frontend.php | 2 +- statify.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/inc/class-statify-frontend.php b/inc/class-statify-frontend.php index bd50eb0..6932211 100644 --- a/inc/class-statify-frontend.php +++ b/inc/class-statify-frontend.php @@ -384,7 +384,7 @@ public static function wp_footer() { wp_enqueue_script( 'statify-js', - plugins_url( 'js/snippet.js', STATIFY_FILE ), + plugins_url( 'js/snippet.min.js', STATIFY_FILE ), array(), STATIFY_VERSION, true diff --git a/statify.php b/statify.php index dd8525a..875222e 100644 --- a/statify.php +++ b/statify.php @@ -20,7 +20,7 @@ define( 'STATIFY_FILE', __FILE__ ); define( 'STATIFY_DIR', dirname( __FILE__ ) ); define( 'STATIFY_BASE', plugin_basename( __FILE__ ) ); -define( 'STATIFY_VERSION', '1.6.3' ); +define( 'STATIFY_VERSION', '1.7.0' ); /* Hooks */