PHP 5.3.15 の CVE-2012-2688 と bug #62432


PHP の 5.4.5 と 5.3.15 がリリースされましたが、幾つか気になる変更点について内容を確認してみました。

Fixed potential overflow in _php_stream_scandir, CVE-2012-2688

修正差分は次の通りです。

PHP Git Repositories - php-src.git/blobdiff - main/streams/streams.c


ソースコードは次の通りで、_php_stream_scandir が修正されています。

PHP Git Repositories - php-src.git/blob - main/streams/streams.c

<?php
PHPAPI int _php_stream_scandir(char *dirname, char **namelist[], int flags, php_stream_context *context,
			  int (*compare) (const char **a, const char **b) TSRMLS_DC)
{
	php_stream *stream;
	php_stream_dirent sdp;
	char **vector = NULL;
	unsigned int vector_size = 0;
	unsigned int nfiles = 0;

	if (!namelist) {
		return FAILURE;
	}

	stream = php_stream_opendir(dirname, ENFORCE_SAFE_MODE | REPORT_ERRORS, context);
	if (!stream) {
		return FAILURE;
	}

	while (php_stream_readdir(stream, &sdp)) {
		if (nfiles == vector_size) {
			if (vector_size == 0) {
				vector_size = 10;
			} else {
				vector_size *= 2;
			}
			vector = (char **) safe_erealloc(vector, vector_size, sizeof(char *), 0);
		}

		vector[nfiles] = estrdup(sdp.d_name);

		nfiles++;
		if(vector_size < 10 || nfiles == 0) {
			/* overflow */
			efree(vector);
			return FAILURE;
		}
	}
	php_stream_closedir(stream);

	*namelist = vector;

	if (compare) {
		qsort(*namelist, nfiles, sizeof(char *), (int(*)(const void *, const void *))compare);
	}
	return nfiles;
}

ファイル名を格納する配列(vector)のサイズ(vector_size)やファイル数(nfiles)が、整数値をオーバーフローした場合の例外が追加されています*1


現実にはなかなか発生しなさそうですが・・・なので「potential」なのでしょう。

Fixed bug #62432 (ReflectionMethod random corrupt memory on high concurrent)

リフレクションに限らず多くのエクステンションのソースが修正されていますがその内容はほとんど同じです。pdo_stmt だと次の通りです。

PHP Git Repositories - php-src.git/blobdiff - ext/pdo/pdo_stmt.c


オブジェクトの生成時の関数(pdo_dbstmt_new)で、クラスエントリ(ce)のデフォルトプロパティ(default_properties)を、クラスのインスタンス(stmt)のプロパティ(properties)にコピーするときのコールバック関数が、zval_add_ref から zval_property_ctor に変更されています。


それぞれの関数の内容は次の通りです。

zval_add_ref
<?php
ZEND_API void zval_add_ref(zval **p)
{
	Z_ADDREF_PP(p);
}
zval_property_ctor
<?php
ZEND_API void zval_property_ctor(zval **p) /* {{{ */
{
	zval *orig_ptr = *p;

	ALLOC_ZVAL(*p);
	**p = *orig_ptr;
	zval_copy_ctor(*p);
	Z_SET_REFCOUNT_PP(p, 1);
	Z_UNSET_ISREF_PP(p);
}
/* }}} */


zval_add_ref は単に参照カウンタをインクリメントしているのに対して、zval_property_ctor では新たな zval インスタンスが作成されています。

何が修正されたか

殆ど私の勘なので間違っている可能性がありますが・・・


PHPでマルチスレッドが有効な場合 pdo_dbstmt_new も複数スレッドから同時に呼ばれることがあります。一方、クラスエントリは複数のスレッドで共有されるリソースです。


zval_add_ref の場合、スレッド間で共有されているリソースであるクラスエントリのデフォルトプロパティの参照カウンタをインクリメントしているので、この処理は複数のスレッドで同時に実行されてはなりません。が、特にその対策が行われていないので、参照カウンタの値がズレる可能性があります。実際の参照数と参照カウンタが一致しなくなるため、メモリの二重解放が行われてしまいます。


zval_property_ctor ではクラスエントリのデフォルトプロパティには変更を行わずにコピーしているため、そのような問題は発生しません。


PHP がマルチスレッドの場合だけの問題なので、Apache prefork や CLI ではこの問題は発生しません。

*1:vector_size の最上位ビットを見たほうがいい気もする???