今時、ファイルアップロードに脆弱であることは珍しいので WordPress MailPoet (wysija-newsletters) Unauthenticated file Upload として知られている脆弱性を調べてみました。
事前に読んだ情報ではzipファイルがアップロードでき、このためにページの改ざんやコード実行ができる旨の解説がされていました。zipファイルでページ改ざん、で何となくは原因と対策は分かります。 VCSのログを見れば簡単に原因が分かるだろう、と思ったのですがどうも公開されているSVNのコミットはまとめてコミットしているらしく、結構差分がありました。 そこでMetasploitのプラグインを見てみました。
uri = normalize_uri(target_uri.path, 'wp-admin', 'admin-post.php')
data = Rex::MIME::Message.new
data.add_part(zip_content, 'application/x-zip-compressed', 'binary', "form-data; name=\"my-theme\"; filename=\"#{rand_text_alpha(5)}.zip\"")
data.add_part('on', nil, nil, 'form-data; name="overwriteexistingtheme"')
data.add_part('themeupload', nil, nil, 'form-data; name="action"')
data.add_part('Upload', nil, nil, 'form-data; name="submitter"')
post_data = data.to_s
payload_uri = normalize_uri(target_uri.path, 'wp-content', 'uploads', 'wysija', 'themes', theme_name, payload_name)
print_status("#{peer} - Uploading payload to #{payload_uri}")
res = send_request_cgi({
'method' => 'POST',
'uri' => uri,
'ctype' => "multipart/form-data; boundary=#{data.bound}",
'vars_get' => { 'page' => 'wysija_campaigns', 'action' => 'themes' },
'data' => post_data
})
やはりプラグインのテーマをアップロードしているようです。 上記の情報を元に差分を見てみると直ぐに原因と修正箇所が分かりました。
diff -urb wysija-newsletters-2.6.6/helpers/back.php wysija-newsletters-2.6.7/helpers/back.php
--- wysija-newsletters-2.6.6/helpers/back.php 2014-05-01 00:18:17.000000000 +0900
+++ wysija-newsletters-2.6.7/helpers/back.php 2014-07-01 10:23:32.000000000 +0900
@@ -46,6 +46,7 @@
}else{
if(WYSIJA_ITF) {
+ add_action('admin_init', array( $this , 'verify_capability'),1);
add_action('admin_init', array($this->controller, 'main'));
add_action('after_setup_theme',array($this,'resolveConflicts'));
}
@@ -172,6 +173,43 @@
}
/**
+ * this function will check the role of the user executing the action, if it's called from another
+ * WordPress admin page than page.php for instance admin-post.php
+ * @return boolean
+ */
+ function verify_capability(){
+ if( isset( $_REQUEST['page'] ) && substr( $_REQUEST['page'] ,0 ,7 ) == 'wysija_' ){
+
+ switch( $_REQUEST['page'] ){
+ case 'wysija_campaigns':
+ $role_needed = 'wysija_newsletters';
+ break;
+ case 'wysija_subscribers':
+ $role_needed = 'wysija_subscribers';
+ break;
+ case 'wysija_config':
+ $role_needed = 'wysija_config';
+ break;
+ case 'wysija_statistics':
+ $role_needed = 'wysija_stats_dashboard';
+ break;
+ default:
+ $role_needed = 'switch_themes';
+ }
+
+ if( current_user_can( $role_needed ) ){
+ return true;
+ } else{
+ die( 'You are not allowed here.' );
+ }
+
+ }else{
+ // this is not a wysija interface/action we can let it pass
+ return true;
+ }
+ }
+
+ /**
* translatable strings need to be not loaded to early, this is why we put them ina separate function
* @global type $wysija_installing
*/
diff -urb wysija-newsletters-2.6.6/helpers/backloader.php wysija-newsletters-2.6.7/helpers/backloader.php
--- wysija-newsletters-2.6.6/helpers/backloader.php 2014-05-01 00:18:17.000000000 +0900
+++ wysija-newsletters-2.6.7/helpers/backloader.php 2014-07-01 10:23:32.000000000 +0900
@@ -24,7 +24,6 @@
/* default script on all wysija interfaces in admin */
wp_enqueue_script( 'wysija-admin-if ', WYSIJA_URL . 'js/admin-wysija.js', array( 'jquery' ), WYSIJA::get_version() );
-
// TO IMPROVE: This has NOTHING TO DO HERE. It has to be moved to the subscribers controller
if ( ! $controller->jsTrans ) {
$controller->jsTrans['selecmiss'] = __( 'Please make a selection first!', WYSIJA );
@@ -67,13 +66,77 @@
$possibleParameters = array( array( $pagename ), array( $pagename, $action ) );
$enqueueFileTypes = array( 'wp_enqueue_script' => array( 'js' => 'js', 'php' => 'js' ), 'wp_enqueue_style' => array( 'css' => 'css' ) );
+ // Files that we have, don't use file_exists if we know which files we have
+ $files = (object) array(
+ 'css' => array(
+ 'add-ons',
+ 'admin-campaigns-articles',
+ 'admin-campaigns-autopost',
+ 'admin-campaigns-bookmarks',
+ 'admin-campaigns-dividers',
+ 'admin-campaigns-editDetails',
+ 'admin-campaigns-editTemplate',
+ 'admin-campaigns-medias',
+ 'admin-campaigns-themes',
+ 'admin-campaigns-viewstats',
+ 'admin-campaigns-welcome_new',
+ 'admin-campaigns',
+ 'admin-global',
+ 'admin-premium',
+ 'admin-statistics',
+ 'admin-subscribers-addlist',
+ 'admin-subscribers-edit',
+ 'admin-subscribers-export',
+ 'admin-subscribers-exportlist',
+ 'admin-subscribers-import',
+ 'admin-subscribers-importmatch',
+ 'admin-subscribers-lists',
+ 'admin-widget',
+ 'admin-config',
+ 'admin-config-form_widget_settings',
+ ),
+ 'js' => array(
+ 'admin-ajax-proto',
+ 'admin-ajax',
+ 'admin-campaigns-articles',
+ 'admin-campaigns-autopost',
+ 'admin-campaigns-bookmarks',
+ 'admin-campaigns-default',
+ 'admin-campaigns-dividers',
+ 'admin-campaigns-edit',
+ 'admin-campaigns-editAutonl',
+ 'admin-campaigns-editDetails',
+ 'admin-campaigns-editTemplate',
+ 'admin-campaigns-image_data',
+ 'admin-campaigns-medias',
+ 'admin-campaigns-themes',
+ 'admin-campaigns-viewstats',
+ 'admin-campaigns-welcome_new',
+ 'admin-config-form_widget_settings',
+ 'admin-config-settings',
+ 'admin-global',
+ 'admin-listing',
+ 'admin-statistics-filter',
+ 'admin-statistics',
+ 'admin-subscribers-edit-manual',
+ 'admin-subscribers-export',
+ 'admin-subscribers-import',
+ 'admin-subscribers-importmatch',
+ 'admin-subscribers',
+ 'admin-tmce',
+ 'admin-wysija-global',
+ 'admin-wysija',
+ )
+ );
+
foreach ( $possibleParameters as $params ) {
- foreach ( $enqueueFileTypes as $wayToInclude => $fileTypes ) {
- foreach ( $fileTypes as $fileType => $folderLocation ){
- if ( file_exists( $dirname . $folderLocation . DS . 'admin-' . implode( '-', $params ) . '.' . $fileType ) ) {
- $sourceIdentifier = 'wysija-autoinc-' . $extension . '-admin-' . implode( '-', $params ) . '-' . $fileType;
- $sourceUrl = $urlname . $folderLocation . '/admin-' . implode( '-', $params ) . '.' . $fileType;
- call_user_func_array( $wayToInclude, array( $sourceIdentifier, $sourceUrl, array(), WYSIJA::get_version() ) );
+ foreach ( $enqueueFileTypes as $method => $types ) {
+ foreach ( $types as $file_type => $file_ext ){
+ $file_slug = 'admin-' . implode( '-', $params );
+ if ( in_array( $file_slug, $files->{ $file_ext } ) ) {
+ $file_id = "wysija-autoinc-{$extension}-{$file_slug}-{$file_ext}";
+ $file_url = "{$urlname}{$file_ext}/{$file_slug}.{$file_ext}";
+ call_user_func_array( $method, array( $file_id, $file_url, array(), WYSIJA::get_version() ) );
}
}
}
@@ -236,7 +299,6 @@
この差分からアクションを実行するロールであるかの確認、アップロードされたZIPアーカイブが妥当であるかのバリデーションが追加されています。
MetasploiteのプラグインはテーマとしてアップロードされるZIPファイルに攻撃用となるPHPスクリプトを入れています。WordPressのMailPoetがロールの確認を怠っていたため、管理者以外でもテーマのZIPファイルがアップロード可能となり、不正なファイルを含むZIPファイルが展開され配置されてしまっていた事が問題の原因です。 対策としてロールの確認、ZIPファイルコンテンツのバリデーションが追加されています。
まとめ
WordPressプラグインでテーマやプラグインなどを扱う場合は必ずロールの確認、アップロードされたZIPファイルのコンテンツを確認しましょう。そうすればこのような致命的な脆弱性を防止できます。
権限確認漏れはそれ自体が致命的な問題です。入力データのバリデーションはセキュリティ対策の基本中の基本です。この脆弱性もZIPファイルの内容をバリデーションしていれば、権限の確認漏れがあっても少しは問題のダメージを低減可能でした。
今日たまたま「WordPressを使わない」というセキュリティ対策をオプションとして推奨する記事を読んだのですが、今回のような脆弱性はWordPressでなくても、プラグインアーキテクチャのシステムであれば起こりえる問題です。
人気の高いソフトウェアは脆弱性が発見された場合の影響範囲は大きいですが、人気の無いソフトウェアの脆弱性はあまり話題にならず、放置され続けることも非常に多いです。広く利用されているソフトウェア、あまり利用されていないソフトウェア、それぞれ良い面、悪い面があるので一概に「広く利用されているソフトウェアが危険（リスクが高い）」とも言えません。あまり利用されていないソフトウェアはあまりレビューされておらず、危険であることも多いです。