Refs #3456, add extra security for GeoIP auto-updater/downloader & fix a couple bugs in the code. Also made the UX a tiny bit more robust.

git-svn-id: http://dev.piwik.org/svn/trunk@7576 59fd770c-687e-43c8-a1e3-f5a4ff64c105

5 files changed, 163 insertions, 9 deletions

diff --git a/plugins/UserCountry/GeoIPAutoUpdater.php b/plugins/UserCountry/GeoIPAutoUpdater.php
index 1d87cd6436..aac72ec5bd 100755
--- a/plugins/UserCountry/GeoIPAutoUpdater.php
+++ b/plugins/UserCountry/GeoIPAutoUpdater.php
@@ -29,6 +29,14 @@ class Piwik_UserCountry_GeoIPAutoUpdater
 		'isp' => self::ISP_URL_OPTION_NAME,
 		'org' => self::ORG_URL_OPTION_NAME,
 	);
+	
+	/**
+	 * PHP Error caught through a custom error handler while trying to use a downloaded
+	 * GeoIP database. See catchGeoIPError for more info.
+	 * 
+	 * @var array
+	 */
+	private static $unzipPhpError = null;
 
 	/**
 	 * Attempts to download new location, ISP & organization GeoIP databases and
@@ -122,7 +130,11 @@ class Piwik_UserCountry_GeoIPAutoUpdater
 	public static function unzipDownloadedFile( $path, $unlink = false )
 	{
 		$parts = explode('.', basename($path));
-		$outputPath = Piwik_UserCountry_LocationProvider_GeoIp::getPathForGeoIpDatabase($parts[0].'.dat');
+		$filenameStart = $parts[0];
+		
+		$dbFilename = $filenameStart.'.dat';
+		$tempFilename = $filenameStart.'.dat.new';
+		$outputPath = Piwik_UserCountry_LocationProvider_GeoIp::getPathForGeoIpDatabase($tempFilename);
 		
 		// extract file
 		if (substr($path, -7, 7) == '.tar.gz')
@@ -141,7 +153,7 @@ class Piwik_UserCountry_GeoIPAutoUpdater
 			foreach ($content as $info)
 			{
 				$archivedPath = $info['filename'];
-				if (basename($archivedPath) === basename($outputPath))
+				if (basename($archivedPath) === $dbFilename)
 				{
 					$datFile = $archivedPath;
 				}
@@ -150,7 +162,7 @@ class Piwik_UserCountry_GeoIPAutoUpdater
 			if ($datFile === null)
 			{
 				throw new Exception(Piwik_Translate('UserCountry_CannotFindGeoIPDatabaseInArchive',
-					array(basename($outputPath), "'$path'")));
+					array($dbFilename, "'$path'")));
 			}
 			
 			// extract JUST the .dat file
@@ -184,6 +196,59 @@ class Piwik_UserCountry_GeoIPAutoUpdater
 			throw new Exception(Piwik_Translate('UserCountry_UnsupportedArchiveType', "'$ext'"));
 		}
 		
+		// test that the new archive is a valid GeoIP database
+		$dbType = Piwik_UserCountry_LocationProvider_GeoIp::getGeoIPDatabaseTypeFromFilename($dbFilename);
+		if ($dbType === false) // sanity check
+		{
+			throw new Exception("Unexpected GeoIP archive file name '$path'.");
+		}
+		
+		$customDbNames = array(
+			'loc' => array(),
+			'isp' => array(),
+			'org' => array()
+		);
+		$customDbNames[$dbType] = array($tempFilename);
+		
+		$phpProvider = new Piwik_UserCountry_LocationProvider_GeoIp_Php($customDbNames);
+		
+		// note: in most cases where this will fail, the error will usually be a PHP fatal error/notice.
+		// in order to delete the files in such a case (which can be caused by a man-in-the-middle attack)
+		// we need to catch them, so we set a new error handler.
+		self::$unzipPhpError = null;
+		set_error_handler(array('Piwik_UserCountry_GeoIPAutoUpdater', 'catchGeoIPError'));
+		
+		$location = $phpProvider->getLocation(array('ip' => Piwik_UserCountry_LocationProvider_GeoIp::TEST_IP));
+		
+		restore_error_handler();
+		
+		if (empty($location)
+			|| self::$unzipPhpError !== null)
+		{
+			if (self::$unzipPhpError !== null)
+			{
+				list($errno, $errstr, $errfile, $errline) = self::$unzipPhpError;
+				Piwik::log("Piwik_UserCountry_GeoIPAutoUpdater: Encountered PHP error when testing newly downloaded".
+					" GeoIP database: $errno: $errstr on line $errline of $errfile.");
+			}
+			
+			// remove downloaded files in this case
+			unlink($outputPath);
+			unlink($path);
+			
+			throw new Exception(Piwik_Translate('UserCountry_ThisUrlIsNotAValidGeoIPDB'));
+		}
+		
+		// delete the existing GeoIP database (if any) and rename the downloaded file
+		$oldDbFile = Piwik_UserCountry_LocationProvider_GeoIp::getPathForGeoIpDatabase($dbFilename);
+		if (file_exists($oldDbFile))
+		{
+			unlink($oldDbFile);
+		}
+		
+		$tempFile = Piwik_UserCountry_LocationProvider_GeoIp::getPathForGeoIpDatabase($tempFilename);
+		rename($existing = $tempFile, $newName = $oldDbFile);
+		
 		// delete original archive
 		if ($unlink)
 		{
@@ -251,7 +316,7 @@ class Piwik_UserCountry_GeoIPAutoUpdater
 		// set url options
 		foreach (self::$urlOptions as $optionKey => $optionName)
 		{
-			if (empty($options[$optionKey]))
+			if (!isset($options[$optionKey]))
 			{
 				continue;
 			}
@@ -317,7 +382,7 @@ class Piwik_UserCountry_GeoIPAutoUpdater
 	 */
 	public static function getConfiguredUrl( $key )
 	{
-		if(!empty(self::$urlOptions[$key])) {
+		if(empty(self::$urlOptions[$key])) {
 			throw new Exception("Invalid key $key");
 		}
 		$url = Piwik_GetOption(self::$urlOptions[$key]);
@@ -361,7 +426,7 @@ class Piwik_UserCountry_GeoIPAutoUpdater
 		$result = array();
 		foreach (self::getConfiguredUrls() as $key => $url)
 		{
-			if ($url !== false)
+			if (!empty($url))
 			{
 				// if a database of the type does not exist, but there's a url to update, then
 				// a database is missing
@@ -398,4 +463,25 @@ class Piwik_UserCountry_GeoIPAutoUpdater
 			return reset($filenameParts);
 		}
 	}
+	
+	/**
+	 * Custom PHP error handler used to catch any PHP errors that occur when
+	 * testing a downloaded GeoIP file.
+	 * 
+	 * If we download a file that is supposed to be a GeoIP database, we need to make
+	 * sure it is one. This is done simply by attempting to use it. If this fails, it
+	 * will most of the time fail as a PHP error, which we catch w/ this function
+	 * after it is passed to set_error_handler.
+	 * 
+	 * The PHP error is stored in self::$unzipPhpError.
+	 * 
+	 * @param int $errno
+	 * @param string $errstr
+	 * @param string $errfile
+	 * @param int $errline
+	 */
+	public static function catchGeoIPError( $errno, $errstr, $errfile, $errline )
+	{
+		self::$unzipPhpError = array($errno, $errstr, $errfile, $errline);
+	}
 }
diff --git a/plugins/UserCountry/LocationProvider/GeoIp.php b/plugins/UserCountry/LocationProvider/GeoIp.php
index 4cb0157169..d9e8f0224f 100755
--- a/plugins/UserCountry/LocationProvider/GeoIp.php
+++ b/plugins/UserCountry/LocationProvider/GeoIp.php
@@ -19,6 +19,7 @@ abstract class Piwik_UserCountry_LocationProvider_GeoIp extends Piwik_UserCountr
 {
 	/* For testing, use: 'http://piwik-team.s3.amazonaws.com/GeoLiteCity.dat.gz' */
 	const GEO_LITE_URL = 'http://geolite.maxmind.com/download/geoip/database/GeoLiteCity.dat.gz';
+	const TEST_IP = '194.57.91.215';
 	
 	public static $geoIPDatabaseDir = 'misc';
 	
@@ -215,7 +216,7 @@ abstract class Piwik_UserCountry_LocationProvider_GeoIp extends Piwik_UserCountr
 			$expected = array(self::COUNTRY_CODE_KEY => 'FR',
 							  self::REGION_CODE_KEY => 'A6',
 							  self::CITY_NAME_KEY => 'Besançon');
-			$result = array('194.57.91.215', $expected);
+			$result = array(self::TEST_IP, $expected);
 		}
 		return $result;
 	}
@@ -231,6 +232,29 @@ abstract class Piwik_UserCountry_LocationProvider_GeoIp extends Piwik_UserCountr
 			|| self::getPathToGeoIpDatabase(self::$dbNames['isp'])
 			|| self::getPathToGeoIpDatabase(self::$dbNames['org']);
 	}
+	
+	/**
+	 * Returns the type of GeoIP database ('loc', 'isp' or 'org') based on the
+	 * filename (eg, 'GeoLiteCity.dat', 'GeoIPISP.dat', etc).
+	 * 
+	 * @param string $filename
+	 * @return string|false 'loc', 'isp', 'org', or false if cannot find a database
+	 *                      type.
+	 */
+	public static function getGeoIPDatabaseTypeFromFilename( $filename )
+	{
+		foreach (self::$dbNames as $key => $names)
+		{
+			foreach ($names as $name)
+			{
+				if ($name === $filename)
+				{
+					return $key;
+				}
+			}
+		}
+		return false;
+	}
 }
 
 
diff --git a/plugins/UserCountry/LocationProvider/GeoIp/Php.php b/plugins/UserCountry/LocationProvider/GeoIp/Php.php
index 63c9e1eb1a..3f1ea53f78 100755
--- a/plugins/UserCountry/LocationProvider/GeoIp/Php.php
+++ b/plugins/UserCountry/LocationProvider/GeoIp/Php.php
@@ -32,6 +32,46 @@ class Piwik_UserCountry_LocationProvider_GeoIp_Php extends Piwik_UserCountry_Loc
 	private $geoIpCache = array();
 	
 	/**
+	 * Possible filenames for each type of GeoIP database. When looking for a database
+	 * file in the 'misc' subdirectory, files with these names will be looked for.
+	 * 
+	 * This variable is an array mapping either the 'loc', 'isp' or 'org' strings with
+	 * an array of filenames.
+	 * 
+	 * By default, this will be set to Piwik_UserCountry_LocationProvider_GeoIp_Php::$dbNames.
+	 * 
+	 * @var array
+	 */
+	private $customDbNames;
+	
+	/**
+	 * Constructor.
+	 * 
+	 * @param array|false $customDbNames The possible filenames for each type of GeoIP database.
+	 *                                   eg array(
+	 *                                       'loc' => array('GeoLiteCity.dat'),
+	 *                                       'isp' => array('GeoIP.dat', 'GeoIPISP.dat')
+	 *                                       'org' => array('GeoIPOrg.dat')
+	 *                                   )
+	 *                                   If a key is missing (or the parameter not supplied), then the
+	 *                                   default database names are used.
+	 */
+	public function __construct( $customDbNames = false )
+	{
+		$this->customDbNames = parent::$dbNames;
+		if ($customDbNames !== false)
+		{
+			foreach ($this->customDbNames as $key => $names)
+			{
+				if (isset($customDbNames[$key]))
+				{
+					$this->customDbNames[$key] = $customDbNames[$key];
+				}
+			}
+		}
+	}
+	
+	/**
 	 * Closes all open geoip instances.
 	 */
 	public function __destruct()
@@ -140,7 +180,7 @@ class Piwik_UserCountry_LocationProvider_GeoIp_Php extends Piwik_UserCountry_Loc
 	 */
 	public function isAvailable()
 	{
-		$path = self::getPathToGeoIpDatabase(parent::$dbNames['loc']);
+		$path = self::getPathToGeoIpDatabase($this->customDbNames['loc']);
 		return $path !== false;
 	}
 	
@@ -303,7 +343,7 @@ class Piwik_UserCountry_LocationProvider_GeoIp_Php extends Piwik_UserCountry_Loc
 			parent::getRegionNames();
 			require_once PIWIK_INCLUDE_PATH . '/libs/MaxMindGeoIP/geoipcity.inc';
 			
-			$pathToDb = self::getPathToGeoIpDatabase(parent::$dbNames[$key]);
+			$pathToDb = self::getPathToGeoIpDatabase($this->customDbNames[$key]);
 			if ($pathToDb !== false)
 			{
 				$this->geoIpCache[$key] = geoip_open($pathToDb, GEOIP_STANDARD); // TODO support shared memory
diff --git a/plugins/UserCountry/templates/admin.js b/plugins/UserCountry/templates/admin.js
index 1077039cba..fc5e71c762 100755
--- a/plugins/UserCountry/templates/admin.js
+++ b/plugins/UserCountry/templates/admin.js
@@ -102,6 +102,9 @@ $(document).ready(function() {
 				}
 			}
 		});
+		ajaxRequest.setErrorCallback(function() {
+			callback({error: _pk_translate('UserCountry_FatalErrorDuringDownload_js')});
+		});
         ajaxRequest.send(false);
 	};
 	
diff --git a/plugins/UserCountry/templates/adminIndex.tpl b/plugins/UserCountry/templates/adminIndex.tpl
index ecf51a265d..7c5fe69131 100755
--- a/plugins/UserCountry/templates/adminIndex.tpl
+++ b/plugins/UserCountry/templates/adminIndex.tpl
@@ -1,3 +1,4 @@
+{loadJavascriptTranslations plugins='UserCountry'}
 {assign var=showSitesSelection value=false}
 {assign var=showPeriodSelection value=false}
 {include file="CoreAdminHome/templates/header.tpl"}