Protect HistoryBlob storage against malicious class injection

* Add a safe unserialize() wrapper for HistoryBlob classes
* Add a safe unserialize() wrapper for plain array data as used for
  compressed internal storage by ConcatenatedGzipHistoryBlob and
  DiffHistoryBlob.
* Fix tests broken by this.
* Fix unnecessary call to uncompress(), __wakeup() does this already.
  Was a phan error now that we have more information about the type of
  $obj.
* Add tests for successful unserialize and wakeup of WMF production
  data.

Change-Id: Ic995dda16d9c6045b33f2fdae7f6575ac8329976

9 files changed, 132 insertions, 21 deletions

diff --git a/autoload.php b/autoload.php
index a10285b61e0b..9d4bd8889a8c 100644
--- a/autoload.php
+++ b/autoload.php
@@ -614,6 +614,7 @@ $wgAutoloadLocalClasses = [
 	'HistoryBlob' => __DIR__ . '/includes/historyblob/HistoryBlob.php',
 	'HistoryBlobCurStub' => __DIR__ . '/includes/historyblob/HistoryBlobCurStub.php',
 	'HistoryBlobStub' => __DIR__ . '/includes/historyblob/HistoryBlobStub.php',
+	'HistoryBlobUtils' => __DIR__ . '/includes/historyblob/HistoryBlobUtils.php',
 	'HistoryPager' => __DIR__ . '/includes/actions/pagers/HistoryPager.php',
 	'Hooks' => __DIR__ . '/includes/Hooks.php',
 	'Html' => __DIR__ . '/includes/Html.php',
diff --git a/includes/Storage/SqlBlobStore.php b/includes/Storage/SqlBlobStore.php
index 58e2565d235b..feff604516fc 100644
--- a/includes/Storage/SqlBlobStore.php
+++ b/includes/Storage/SqlBlobStore.php
@@ -28,6 +28,7 @@ namespace MediaWiki\Storage;
 use AppendIterator;
 use DBAccessObjectUtils;
 use ExternalStoreAccess;
+use HistoryBlobUtils;
 use IDBAccessObject;
 use InvalidArgumentException;
 use MWException;
@@ -595,8 +596,8 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
 
 		if ( in_array( 'object', $blobFlags ) ) {
 			# Generic compressed storage
-			$obj = unserialize( $blob );
-			if ( !is_object( $obj ) ) {
+			$obj = HistoryBlobUtils::unserialize( $blob );
+			if ( !$obj ) {
 				// Invalid object
 				return false;
 			}
diff --git a/includes/externalstore/ExternalStoreDB.php b/includes/externalstore/ExternalStoreDB.php
index 870cf2850113..3fbe335fb56b 100644
--- a/includes/externalstore/ExternalStoreDB.php
+++ b/includes/externalstore/ExternalStoreDB.php
@@ -333,7 +333,7 @@ class ExternalStoreDB extends ExternalStoreMedium {
 		}
 		if ( $itemID !== false && $ret !== false ) {
 			// Unserialise object; caller extracts item
-			$ret = unserialize( $ret );
+			$ret = HistoryBlobUtils::unserialize( $ret );
 		}
 
 		$externalBlobCache = [ $cacheID => $ret ];
@@ -410,7 +410,7 @@ class ExternalStoreDB extends ExternalStoreMedium {
 				$ret[$id] = $row->blob_text;
 			} else {
 				// multi result stored per blob
-				$ret[$id] = unserialize( $row->blob_text );
+				$ret[$id] = HistoryBlobUtils::unserialize( $row->blob_text );
 			}
 		}
 	}
diff --git a/includes/historyblob/ConcatenatedGzipHistoryBlob.php b/includes/historyblob/ConcatenatedGzipHistoryBlob.php
index b227c1e7cf5d..823c8ff6f35a 100644
--- a/includes/historyblob/ConcatenatedGzipHistoryBlob.php
+++ b/includes/historyblob/ConcatenatedGzipHistoryBlob.php
@@ -119,7 +119,7 @@ class ConcatenatedGzipHistoryBlob implements HistoryBlob {
 	 */
 	public function uncompress() {
 		if ( $this->mCompressed ) {
-			$this->mItems = unserialize( gzinflate( $this->mItems ) );
+			$this->mItems = HistoryBlobUtils::unserializeArray( gzinflate( $this->mItems ) );
 			$this->mCompressed = false;
 		}
 	}
diff --git a/includes/historyblob/DiffHistoryBlob.php b/includes/historyblob/DiffHistoryBlob.php
index 8c935163b469..6f7ac43590a7 100644
--- a/includes/historyblob/DiffHistoryBlob.php
+++ b/includes/historyblob/DiffHistoryBlob.php
@@ -322,7 +322,7 @@ class DiffHistoryBlob implements HistoryBlob {
 	public function __wakeup() {
 		// addItem() doesn't work if mItems is partially filled from mDiffs
 		$this->mFrozen = true;
-		$info = unserialize( gzinflate( $this->mCompressed ) );
+		$info = HistoryBlobUtils::unserializeArray( gzinflate( $this->mCompressed ) );
 		$this->mCompressed = null;
 
 		if ( !$info ) {
diff --git a/includes/historyblob/HistoryBlobStub.php b/includes/historyblob/HistoryBlobStub.php
index d05cd822b96e..d485740d5107 100644
--- a/includes/historyblob/HistoryBlobStub.php
+++ b/includes/historyblob/HistoryBlobStub.php
@@ -128,19 +128,13 @@ class HistoryBlobStub {
 			if ( in_array( 'gzip', $flags ) ) {
 				// This shouldn't happen, but a bug in the compress script
 				// may at times gzip-compress a HistoryBlob object row.
-				$obj = unserialize( gzinflate( $row->old_text ) );
+				$obj = HistoryBlobUtils::unserialize( gzinflate( $row->old_text ), true );
 			} else {
-				$obj = unserialize( $row->old_text );
-			}
-
-			if ( !is_object( $obj ) ) {
-				// Correct for old double-serialization bug.
-				$obj = unserialize( $obj );
+				$obj = HistoryBlobUtils::unserialize( $row->old_text, true );
 			}
 
 			// Save this item for reference; if pulling many
 			// items in a row we'll likely use it again.
-			$obj->uncompress();
 			self::$blobCache = [ $this->mOldId => $obj ];
 		}
 
diff --git a/includes/historyblob/HistoryBlobUtils.php b/includes/historyblob/HistoryBlobUtils.php
new file mode 100644
index 000000000000..ea402695538b
--- /dev/null
+++ b/includes/historyblob/HistoryBlobUtils.php
@@ -0,0 +1,56 @@
+<?php
+
+use MediaWiki\Storage\BlobAccessException;
+
+class HistoryBlobUtils {
+	public const NO_CLASSES = [ 'allowed_classes' => false ];
+
+	/**
+	 * Get the classes which are allowed to be contained in a text or ES row
+	 *
+	 * @return string[]
+	 */
+	public static function getAllowedClasses() {
+		return [
+			ConcatenatedGzipHistoryBlob::class,
+			DiffHistoryBlob::class,
+			HistoryBlobCurStub::class,
+			HistoryBlobStub::class
+		];
+	}
+
+	/**
+	 * Unserialize a HistoryBlob
+	 *
+	 * @param string $str
+	 * @param bool $allowDouble Allow double serialization
+	 * @return HistoryBlob|HistoryBlobStub|HistoryBlobCurStub|null
+	 */
+	public static function unserialize( string $str, bool $allowDouble = false ) {
+		$obj = unserialize( $str, [ 'allowed_classes' => self::getAllowedClasses() ] );
+		if ( is_string( $obj ) && $allowDouble ) {
+			// Correct for old double-serialization bug (needed by HistoryBlobStub only)
+			$obj = unserialize( $obj, [ 'allowed_classes' => self::getAllowedClasses() ] );
+		}
+		foreach ( self::getAllowedClasses() as $class ) {
+			if ( $obj instanceof $class ) {
+				return $obj;
+			}
+		}
+		return null;
+	}
+
+	/**
+	 * Unserialize array data with no classes
+	 *
+	 * @param string $str
+	 * @return array
+	 */
+	public static function unserializeArray( string $str ): array {
+		$array = unserialize( $str, [ 'allowed_classes' => false ] );
+		if ( !is_array( $array ) ) {
+			throw new BlobAccessException( "Expected array in serialized string" );
+		}
+		return $array;
+	}
+}
diff --git a/tests/phpunit/includes/Storage/SqlBlobStoreTest.php b/tests/phpunit/includes/Storage/SqlBlobStoreTest.php
index ecf7bb75288a..3e973b6926f9 100644
--- a/tests/phpunit/includes/Storage/SqlBlobStoreTest.php
+++ b/tests/phpunit/includes/Storage/SqlBlobStoreTest.php
@@ -2,6 +2,7 @@
 
 namespace MediaWiki\Tests\Storage;
 
+use ConcatenatedGzipHistoryBlob;
 use ExternalStoreAccess;
 use ExternalStoreFactory;
 use HashBagOStuff;
@@ -9,7 +10,6 @@ use InvalidArgumentException;
 use MediaWiki\Storage\BlobAccessException;
 use MediaWiki\Storage\SqlBlobStore;
 use MediaWikiIntegrationTestCase;
-use TitleValue;
 use WANObjectCache;
 use Wikimedia\Rdbms\LoadBalancer;
 
@@ -84,6 +84,12 @@ class SqlBlobStoreTest extends MediaWikiIntegrationTestCase {
 		$this->assertTrue( $store->getUseExternalStore() );
 	}
 
+	private function makeObjectBlob( $text ) {
+		$obj = new ConcatenatedGzipHistoryBlob();
+		$obj->setText( $text );
+		return serialize( $obj );
+	}
+
 	public function provideDecompress() {
 		yield '(no legacy encoding), empty in empty out' => [ false, '', [], '' ];
 		yield '(no legacy encoding), string in string out' => [ false, 'A', [], 'A' ];
@@ -96,17 +102,16 @@ class SqlBlobStoreTest extends MediaWikiIntegrationTestCase {
 			// gzip string below generated with serialize( 'JOJO' )
 			false, "s:4:\"JOJO\";", [ 'object' ], false,
 		];
+
 		yield '(no legacy encoding), serialized object in with object flag returns string' => [
 			false,
-			// Using a TitleValue object as it has a getText method (which is needed)
-			serialize( new TitleValue( 0, 'HHJJDDFF' ) ),
+			$this->makeObjectBlob( 'HHJJDDFF' ),
 			[ 'object' ],
 			'HHJJDDFF',
 		];
 		yield '(no legacy encoding), serialized object in with object & gzip flag returns string' => [
 			false,
-			// Using a TitleValue object as it has a getText method (which is needed)
-			gzdeflate( serialize( new TitleValue( 0, '8219JJJ840' ) ) ),
+			gzdeflate( $this->makeObjectBlob( '8219JJJ840' ) ),
 			[ 'object', 'gzip' ],
 			'8219JJJ840',
 		];
@@ -124,13 +129,13 @@ class SqlBlobStoreTest extends MediaWikiIntegrationTestCase {
 		];
 		yield '(ISO-8859-1 encoding), serialized object in with object flags returns string' => [
 			'ISO-8859-1',
-			serialize( new TitleValue( 0, iconv( 'utf-8', 'ISO-8859-1', "3®Àþ3" ) ) ),
+			$this->makeObjectBlob( iconv( 'utf-8', 'ISO-8859-1', "3®Àþ3" ) ),
 			[ 'object' ],
 			'3®Àþ3',
 		];
 		yield '(ISO-8859-1 encoding), serialized object in with object & gzip flags returns string' => [
 			'ISO-8859-1',
-			gzdeflate( serialize( new TitleValue( 0, iconv( 'utf-8', 'ISO-8859-1', "2®Àþ2" ) ) ) ),
+			gzdeflate( $this->makeObjectBlob( iconv( 'utf-8', 'ISO-8859-1', "2®Àþ2" ) ) ),
 			[ 'gzip', 'object' ],
 			'2®Àþ2',
 		];
diff --git a/tests/phpunit/unit/includes/historyblob/HistoryBlobUtilsTest.php b/tests/phpunit/unit/includes/historyblob/HistoryBlobUtilsTest.php
new file mode 100644
index 000000000000..502d3ed6872e
--- /dev/null
+++ b/tests/phpunit/unit/includes/historyblob/HistoryBlobUtilsTest.php
@@ -0,0 +1,54 @@
+<?php
+
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @covers HistoryBlobUtils
+ */
+class HistoryBlobUtilsTest extends TestCase {
+	public static function provideUnserialize() {
+		return [
+			'old production HBS' => [
+				'O:15:"historyblobstub":2:{s:6:"mOldId";s:7:"5817087";s:5:"mHash";s:32:"d41d8cd98f00b204e9800998ecf8427e";}',
+				HistoryBlobStub::class
+			],
+			'new HBS' => [
+				'O:15:"HistoryBlobStub":3:{s:9:"\000*\000mOldId";s:1:"1";s:8:"\000*\000mHash";s:32:"69f6b94cbc2a253d1ca928853bafebfb";s:7:"\000*\000mRef";s:1:"5";}',
+				HistoryBlobStub::class
+			],
+			'old production HBCS' => [
+				'O:18:"historyblobcurstub":1:{s:6:"mCurId";i:8;}',
+				HistoryBlobCurStub::class,
+			],
+			'old production CGZ' => [
+				'O:27:"concatenatedgziphistoryblob":4:{s:8:"mVersion";i:0;s:11:"mCompressed";b:1;s:6:"mItems";s:114:"M\3141\016\2030\f\000\300\257T\351\ab\'N"\363\201>\240\033bp\354 \245\035\220\240\033\352\337\021L\354\247\023F\3367\016\310\016$\242\2304\210\350\3154y/9\345\242\000\220\233\247\352\206\215\221\330=\327f}m\372\343q|uk\337\376x/\237e\232NpF>j\3049E\v\244T\305\022\005P\250y\256%K\251zEp\217n\215\033\376\a";s:12:"mDefaultHash";s:32:"1a42adae1420ddc600a7678c1117e05b";}',
+				ConcatenatedGzipHistoryBlob::class
+			],
+			'production DHB (PHP 5)' => [
+				'O:15:"DiffHistoryBlob":1:{s:11:"mCompressed";s:415:"\245RM\213\023A\020M\306\333\\\\\325{3\b^V\262I\360\203\n\203\304/\024\027\227hnC\b\025\247&\333\320\323=\366\307\262a\022\360Wx\364\254\177C\217\373\217<Y=\031\024\331\243\r\3254\257_\277\256zU\b\023h\035<\204\254\224U\345\262\031\302\024Z\t\2473\a\323\351\004\262A\277n=\035\016\006m\273<\177>\232\ao,\351\303!M\363\\\\,D\236\247P\024\257I\325\301\213E@\345\245\336\222]\255"\374R)\262;\206\211\364\021y\217\273\332\3502B\2320\034\301\027\250%)\306\244\216\264\a\274\322\033\277\025\305[\364\2645V\022,I*%\235\247}O\030-\370\035SH\303\031\343\302T\002\203\2770\326\211\315Nh\254\tD\2440\303u\f\024%\tr\037\255\214\357\235h\214\025\332\324\033\373\207XY`q\n,\321\240\025\306\226\226\004\252\346\0027?\277{\371)POl<\3145]\231\277\272\030\376\021=Jf3\tc6v\374\370\021d\277\314\235\353\350\351p\261^\277;\347J\327\353\224K\376\0206|\336/\245\'\225\027E\024$\026\264\242/s\265\332\277\321%]\3457n\242C]CBr\227\033\266\344\030\316\377[29\260\316\017\216\230\374\204\223\177\302C\361\352\313\375\366\033\347\236\304\311\270\315\221\214x\373\034\017\327\274=\213_\337\303\n\316vN\\\\\242\026\246w\361\322\250-i\307\335\300\272s$IY\345k\247~\340\201\203\254\306&\233u\363xz2>\0313\374\033";}',
+				DiffHistoryBlob::class
+			],
+			'disallowed class' => [
+				'O:8:"stdClass":0:{}',
+				null
+			],
+		];
+	}
+
+	/**
+	 * @dataProvider provideUnserialize
+	 */
+	public function testUnserialize( $input, $expectedClass ) {
+		$obj = HistoryBlobUtils::unserialize( stripcslashes( $input ) );
+		if ( $expectedClass === null ) {
+			$this->assertNull( $obj );
+		} else {
+			$this->assertInstanceOf( $expectedClass, $obj );
+		}
+	}
+
+	public function testUnserializeBadEmbedded() {
+		$obj = HistoryBlobUtils::unserialize( "O:15:\"HistoryBlobStub\":4:{s:9:\"\0*\0mOldId\";N;s:8:\"\0*\0mHash\";s:0:\"\";s:7:\"\0*\0mRef\";N;s:3:\"bad\";O:8:\"stdClass\":0:{}}" );
+		$this->assertInstanceOf( '__PHP_Incomplete_Class', $obj->bad );
+	}
+}