てきとうなメモ

本の感想とか技術メモとか

ImageMagickの脆弱性(ImageTragick)

piyokangoさんが詳しいが自分も少し調べたので。

d.hatena.ne.jp
ImageTragick

どんな脆弱性

外部からの入力により、意図せずに、ファイルを読みこんだり、ファイルを移動したり、削除したり、特定のURLにアクセスしたり、任意のコードを実行可能な脆弱性

どういうアプリが攻撃される?

画像をアップロードしてImageMagickを使って変換するみたいなサービスが狙われるかな。

どういう仕組みで攻撃しているのか

ImageMagickはいろんな種類のファイルを処理できるが、そのためにcoderとdelegateという機能がある。coderはライブラリとして各種ファイルを変換する機能、delegateは適切なcoderがなかった時に、外部コマンドを用いて変換する機能である。

CVE-2016-3714(コード実行の脆弱性)については、delegateの機能で外部コマンドを呼び出す時にsystem関数を利用していたが、shellの特殊文字のエスケープを実施していなかったことが主な原因。

例えば、

convert https://www.imagemagick.org/image/wizard.png wizard.png

を実行すると、imagemagickのサイトからwizard.pngをダウンロードしてwizard.pngというファイルに保存している。

内部的には次のような処理を行っている。
ImageMagickはconvertの変換元(第一引数)をHTTPSという種類のファイルとみなす。HTTPSのcoderは存在しないので、delegateを探す。delegateはdelegates.xmlに記述されており、CentOS6では以下のものが該当する

<delegate decode="https" command="&quot;curl&quot; -s -k -o &quot;%o&quot; &quot;https:%M&quot;"/>

ImageMagickはcommandの部分のコマンドを実行する。%Mには入力ファイル名が渡される。しかし%Mを置換する際にshellの特殊文字のエスケープを実施していないので、

convert 'https://www.imagemagick.org/image/wizard.png"|ls "-la' wizard.png

を実行すると

"curl" -s -k -o "wizard.png" "https://www.imagemagick.org/image/wizard.png"|ls "-la"

が実行され、lsも実行されることになる。

これだけだと、普通のアプリだと外部の入力をそのままconvertの引数にしないから大丈夫じゃないかと思える。が、この処理を画像ファイルの中に隠すことができる。

ImageMagickにはSVGとMVG(ImageMagick独自のテキストベースの画像フォーマット)のcoderがあり、それらのフォーマットは外部のURLやファイルを参照できる。例えば以下のMVGファイルは外部URLhttps://example.com/image.jpgを含む。

push graphic-context
viewbox 0 0 640 480
fill 'url(https://example.com/image.jpg)'
pop graphic-context

この外部URL参照に対してもcoder/delegateが実行される。今回の場合はHTTPSフォーマットだとみなされ、HTTPSdelegate commandが実行され、任意のプロセスを起動できる。

さらに、ImageMagickはそのファイルがどのフォーマットのファイルかについて

  1. マジックバイト
  2. format:filename形式のprefix
  3. 拡張子(filename.suffix)

の優先順位でチェックする。SVGやMVGはマジックバイトがチェックされるので、拡張子pngなどを指定していたとしても、中身がSVG/MVGならばSVG/MVGと解釈されてしまう。

そのため、MVGファイルをアップロードする→convertでファイルを処理しようとするとMVGのcoderが実行される→URLを参照している部分でコードを実行可能ということになる。

その他の脆弱性についてだが、shellコマンドのエスケープの問題はないものの、これらもMVG内に外部のファイルやURLを参照することにより、ファイル/URLの読み書きを行ってしまうことが原因となっている。

修正版は?

最新版(7.0.1-1)をインストールすると防げるようである。

実際に動かしてみると一部しか防げない(×は防げていない)のでpolicy.xmlで防ぐ必要がある。

CVE 内容 防げる?
CVE-2016-3714 delegateによるRCE脆弱性
CVE-2016-3718 SSRF(URLに対するGET) ×
CVE-2016-3715 ephemeralプロトコルによるファイルの削除 ×
CVE-2016-3716 mslプロトコルによるファイルの移動 *1
CVE-2016-3717 labelプロトコルによるファイルの読み込み ×

回避策は?

脆弱性公式サイトによると以下のどちらかで防げる。

  1. マジックバイトをチェックする
  2. policy.xmlに以下を指定
<policymap>
  <policy domain="coder" rights="none" pattern="EPHEMERAL" />
  <policy domain="coder" rights="none" pattern="URL" />
  <policy domain="coder" rights="none" pattern="HTTPS" />
  <policy domain="coder" rights="none" pattern="MVG" />
  <policy domain="coder" rights="none" pattern="MSL" />
  <policy domain="coder" rights="none" pattern="TEXT" />
  <policy domain="coder" rights="none" pattern="SHOW" />
  <policy domain="coder" rights="none" pattern="WIN" />
  <policy domain="coder" rights="none" pattern="PLT" />
</policymap>

coderとかdelegateの周りの処理の流れがよくわからん

私もよくわかっていないが、以下のような感じかな

  1. 仕組みのところに書いた優先度でフォーマットを決める
    • マジックバイトのチェックはMagicCore/magic.cのMagicMapの値を読むとなんとなくわかる
    • magic.xmlにもカスタマイズしたものを指定可能だけども
  2. フォーマットに対応するcoderを探し、存在すればcoderを実行する
    • どのフォーマットがどのcoderに該当するかはMagicCore/coder.cのCoderMapを読むとなんとなくわかる
  3. coderが存在しなければdelegateを実行する
    • デフォルトのdelegateはconfig/delegates.xml.inに書かれてある

RedHatの回避策は微妙?

ImageMagick Filtering Vulnerability - CVE-2016-3714のResolveのMitigationのことだよね。うん、これは微妙だ。RHEL 5では使えないし、明示的にHTTPとFTPを禁止している理由が判らない。私がコードを眺めた限りでは、HTTPとFTPはURLが提供しているようだ。やれやれ。

policy.xmlが使えない古いImageMagickでImageTragickを回避する (2) - Qiita

私がコードを読んだ感じだとcoderのpolicyチェックはcoder名に対して行うのではなく、フォーマットに対して行うのでそんなに悪くない気がするかな。MagicCore/constitute.cのReadImage関数の部分

  if (IsRightsAuthorized(domain,rights,read_info->magick) == MagickFalse)
    {    
      errno=EPERM;
      (void) ThrowMagickException(exception,GetMagickModule(),PolicyError,
        "NotAuthorized","`%s'",read_info->filename);
      read_info=DestroyImageInfo(read_info);
      return((Image *) NULL);
    }

read_info->magickにはcoder名ではなくフォーマット名が入る。

実際にMVG内にHTTPのURLを書いた場合はHTTPのcoderをnoneにしないと防げなかった。

ただ、そもそもMVGをnoneにした時点で防げる話ではあるのだけども。

修正内容はどんなもの?

いまいち理解しきれていないが今回の脆弱性に関して7.0.1-1までに以下の修正が入ったっぽい。

その後の修正

今回の脆弱性関係でまだいくつか修正しているっぽい。

*1:なぜ防げているのかよくわからない

紙のコミックとKindleコミックで発売日が同じもの 2016年06月分

あせびと空世界の冒険者(5)【特典ペーパー付き】 (RYU COMICS)

あせびと空世界の冒険者(5)【特典ペーパー付き】 (RYU COMICS)

レズと七人の彼女たち(1)

レズと七人の彼女たち(1)

泣かせた責任とってくれ 4 (NextFcomics)

泣かせた責任とってくれ 4 (NextFcomics)

大正ロマンチカ12 (NextFcomics)

大正ロマンチカ12 (NextFcomics)

熱愛プリンス お兄ちゃんはキミが好き4 (NextFcomics)

熱愛プリンス お兄ちゃんはキミが好き4 (NextFcomics)

アクセルスター 2 (ジャンプコミックスDIGITAL)

アクセルスター 2 (ジャンプコミックスDIGITAL)

コネクト 1 (ヤングジャンプコミックスDIGITAL)

コネクト 1 (ヤングジャンプコミックスDIGITAL)

ボクガール 10 (ヤングジャンプコミックスDIGITAL)

ボクガール 10 (ヤングジャンプコミックスDIGITAL)

黒 3 (ヤングジャンプコミックスDIGITAL)

黒 3 (ヤングジャンプコミックスDIGITAL)

はるはなのみの 1 (マーガレットコミックスDIGITAL)

はるはなのみの 1 (マーガレットコミックスDIGITAL)

ほしとくず 1 (マーガレットコミックスDIGITAL)

ほしとくず 1 (マーガレットコミックスDIGITAL)

わたしの上司 3 (マーガレットコミックスDIGITAL)

わたしの上司 3 (マーガレットコミックスDIGITAL)

バディゴ! 5 (りぼんマスコットコミックスDIGITAL)

バディゴ! 5 (りぼんマスコットコミックスDIGITAL)

叶恋どうぶつえん (マーガレットコミックスDIGITAL)

叶恋どうぶつえん (マーガレットコミックスDIGITAL)

斉藤さん もっと! 4 (マーガレットコミックスDIGITAL)

斉藤さん もっと! 4 (マーガレットコミックスDIGITAL)

椿町ロンリープラネット 4 (マーガレットコミックスDIGITAL)

椿町ロンリープラネット 4 (マーガレットコミックスDIGITAL)

Dance with Devils -Blight- 2巻 (デジタル版Gファンタジーコミックス)

Dance with Devils -Blight- 2巻 (デジタル版Gファンタジーコミックス)

Apache Struts2の脆弱性(CVE-2016-3081)

JVNVU#91375252: Apache Struts2 に任意のコード実行の脆弱性

ぐぐるとみつかるPOCで1番簡単そうなのを解説してみる

http://www.example.com/sample.action?method:%23_memberAccess%3D@ognl.OgnlContext@DEFAULT_MEMBER_ACCESS%2C%23test%3D%23context.get%28%23parameters.res%5B0%5D%29.getWriter%28%29%2C%23test.println%28%23parameters.command%5B0%5D%29%2C%23test.flush%28%29%2C%23test.close&res=com.opensymphony.xwork2.dispatcher.HttpServletResponse&command=%23%23%23Struts2%20S2-032%20Vulnerable%23%23%23

脆弱性のあるStruts 2.3.28で作成したActionに対しDMIを有効にした状態でStrutsのActionのURLに上記のようなクエリパラメタつけてアクセスすると

###Struts2 S2-032 Vulnerable###

と表示される。

DMIって何かというとmethod:XXXとクエリパラメタに与えて上げると、ActionのXXXメソッドを実行してくれるというもの。単純にメソッドとして評価すれば良いのだが、何故かOGNL(Javaのオブジェクトを操作可能な簡易言語)を記述可能である。

上記POCのmethod:以下の部分をURLデコードすると以下になる

method:#_memberAccess=@ognl.OgnlContext@DEFAULT_MEMBER_ACCESS,#test=#context.get(#parameters.res[0]).getWriter(),#test.println(#parameters.command[0]),#test.flush(),#test.close&res=com.opensymphony.xwork2.dispatcher.HttpServletResponse&command=###Struts2 S2-032 Vulnerable###

OGNLを知らないと分かりづらいので、これを1つずつ説明する。

#_memberAccess=@ognl.OgnlContext@DEFAULT_MEMBER_ACCESS,

#は変数への代入や参照時に使う記号であり、@class@fieldは静的フィールドclass.fieldを取得する。_memberAccessはognl.OgnlContextのフィールドであり、OGNLを評価する時にどのような処理を許すかどうかを設定する。普通にアクセスしようとすると特定のクラスやパッケージへのアクセスを除外するようになっている。struts.xmlstruts.excludedClassesなどで指定している。例えば、java.lang.Runtimeなどにはアクセスできないようだ。

#test=#context.get(#parameters.res[0]).getWriter(),

parametersはクエリパラメタを表す。そのため、parameters.res[0]はcom.opensymphony.xwork2.dispatcher.HttpServletResponseである。contextはOGNLのコンテキストOgnlContextであり、コンテキストにおけるキーcom.opensymphony.xwork2.dispatcher.HttpServletResponseに対応する値はサーブレットのレスポンスオブジェクトでありそのWriterを取得している。

#test.println(#parameters.command[0]),
#test.flush(),
#test.close

クエリパラメタcommandの値を出力して、Writerをflush,closeしている。最後に()がついていないのはこれがDMIであるので元々XXXを指定した時にXXX()になるように()をくっつけるコードになっているためである。

で、今回の修正はOGNLとして評価する部分は修正しておらず、DMIで実行するメソッドのクリーンを実施している。

--- a/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java
+++ b/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java
@@ -136,7 +136,7 @@ public class DefaultActionMapper implements ActionMapper {
                 put(METHOD_PREFIX, new ParameterAction() {
                     public void execute(String key, ActionMapping mapping) {
                         if (allowDynamicMethodCalls) {
-                            mapping.setMethod(key.substring(METHOD_PREFIX.length()));
+                            mapping.setMethod(cleanupActionName(key.substring(METHOD_PREFIX.length())));
                         }
                     }
                 });
@@ -148,7 +148,7 @@ public class DefaultActionMapper implements ActionMapper {
                             if (allowDynamicMethodCalls) {
                                 int bang = name.indexOf('!');
                                 if (bang != -1) {
-                                    String method = name.substring(bang + 1);
+                                    String method = cleanupActionName(name.substring(bang + 1));
                                     mapping.setMethod(method);
                                     name = name.substring(0, bang);
                                 }

cleanupActionNameは以下のようなメソッド

    /** 
     * Cleans up action name from suspicious characters
     *
     * @param rawActionName action name extracted from URI
     * @return safe action name
     */
    protected String cleanupActionName(final String rawActionName) {
        if (allowedActionNames.matcher(rawActionName).matches()) {
            return rawActionName;
        } else {
            LOG.warn("Action [{}] does not match allowed action names pattern [{}], cleaning it up!",
                    rawActionName, allowedActionNames);
            String cleanActionName = rawActionName;
            for (String chunk : allowedActionNames.split(rawActionName)) {
                cleanActionName = cleanActionName.replace(chunk, "");
            }   
            LOG.debug("Cleaned action name [{}]", cleanActionName);
            return cleanActionName;
        }   
    } 

allowedActionNamesは[a-zA-Z0-9._!/\\-]*なので今回のOGNL式からは#や()などが削除される。そのためまともなOGNLとして評価されず例外が発生するようになっている。

ソースコードにコメントを書くかどうか

プログラムにコメント書かない文化もあるよって話 - NZ MoyaSystem

一部同意できる部分もあるのだけども、やっぱりある程度コメントは書いたほうが良いと思う。書きすぎはよくないが。

自分が書いたほうが良いと思うのは以下の部分かな

  • 公開している関数/定数
  • 複雑なアルゴリズムを実装している部分
  • なぜこのコードなのかを書いたほうが良い部分
    • 外部ライブラリのバグなどで、おかしなコードになっているけど正しいコード
    • ある仕様に決めたけども、いろいろ議論がありうるもの

「なぜ」の部分も書いてもらった方が、仕様を変えるかどうか迷った時に参考になる。

きれいなコードを書けばコメント書かなくても良いという意見だけども、そもそもその綺麗さにはプログラミング言語/ライブラリで書けることという限界値があって、現在のプログラミング言語/ライブラリはそこまで完璧なものだとは思っていない。ので、どうしてもコメント書かないとわからない部分がでてくるよなと。

また、読む側の視点がぬけているよね。コメントはあったほうが読む速度があがるだろうし、邪魔にならない程度ならばあった方が良いだろう。ある程度のコードを読む能力が高い人を常に確保できるのであれば省略しても良いけど現実は結構難しい。書く側がこれぐらい読めるよと思っていても、それは常に読んでいるからバイアスがかかっていて、読む側は内心文句言いながら読んでいるかもしれない。

あと気になった部分。

プログラムの仕様書にあたるドキュメントについては、詳細かつ簡潔(1チームにつき Word 数十ページ程度)にまとまったものが共有されており、仕様変更が発生した場合には随時メンテナンスが入っています。コーディング担当者がメンテを忘れても、レビュー時にたいてい誰かが指摘してくれるので、メンテナンス漏れが起きることは少ないです。

仕様書のメンテナンスコストがコメントのメンテナンスコストって同じじゃないのかな。どこに書くかが違うだけで。

結局コメント書いてるじゃんと思われるかもしれませんが、「なぜこの修正を行ったのか」「修正前後で動作がどう違うのか」などの情報は、プログラムそのものではなくコミットやチケットに付随する情報のため、プログラムのコメントとして残すのは不適切です。

「なぜこの修正を行ったのか」「修正前後で動作がどう違うか」はコミットログで良いと思うが、「現状なぜこのコードになっているのか」はソースコードに書くべき話かな。

--no-preserve-rootオプション

サーバ業者が" rm -rf / "で全サーバを誤消去、復旧法をQ&Aサイトに尋ねる。実は書籍執筆のための「引っ掛け問題」 - Engadget Japanese

種明かしをすると、現在の UNIX(Linux)システムでは管理者権限で "rm -rf / " を実行しても警告メッセージが表示され、すぐにはファイルを削除しないようになっています。そのうえで、本当に削除をするなら"-no preserve"オプションを付けて実行することになっています。このことをどれだけのユーザーが知っているか、マルサラは試したわけです。このとき、質問へのアクセス数は述べ14万に達していましたが、このことを指摘する人はほとんどいませんでした。

--no-preserve-rootオプションのことだと思うけども、そういえばそんなオプションあったなあ。

これいつからなんだろと調べてみるとcoreutilsChangelogを調べると結構古い

2003-11-09 Jim Meyering

* Use automake-1.7.9. Regenerate dependent files.

* src/rm.c: Support new options: --preserve-root and --no-preserve-root.
* src/chown.c: Likewise.

http://git.savannah.gnu.org/cgit/coreutils.git/tree/ChangeLog-2005#n7384

2003-12-20 Jim Meyering

* Version 5.1.0.

http://git.savannah.gnu.org/cgit/coreutils.git/tree/ChangeLog-2005#n7238

どうも2003年末の5.1.0かららしい。

ちなみにrmだけでなくchownやchgrpも--no-preserve-rootオプションがある。所有者/グループを全部変えられると動かなくなるしなあ

suやsudoにおけるPATH

suとかsudoとかrootになる方法はいくつかあるけど、/usr/local/binにパスが通っていないことがあった。
で、なんでかなと。

centos7の環境で確認すると以下のようになる。

$ echo $PATH
/usr/local/bin:/usr/bin:/usr/local/sbin:/usr/sbin:/home/vagrant/.local/bin:/home/vagrant/bin
$ su
# echo $PATH
/usr/local/bin:/usr/bin:/usr/local/sbin:/usr/sbin:/home/vagrant/.local/bin:/home/vagrant/bin
# exit
$ su -
# echo $PATH
/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin:/root/bin
# exit
$ sudo -s
# echo $PATH
/sbin:/bin:/usr/sbin:/usr/bin
# exit
$ sudo -i
# echo $PATH
/usr/local/sbin:/sbin:/bin:/usr/sbin:/usr/bin:/root/bin
# exit

結構違う。

まず、suした時はsuを実行したユーザのPATHを引き継ぐ。

su -した時は以下manにあるように

-, -l, --login
シェルをログインシェルにする。すなわち以下のような取り扱いをする: すべての環境変数を解除する。その上で `TERM'、 `HOME'、 `SHELL' を前述 のように設定し、
`USER'、 `LOGNAME' (スーパーユーザーであっても)を同 じく前述のように設定する。続いて `PATH' をコンパイル時のデフォルト値に 設定する。ディレクトリを user
のホームディレクトリに変更する。シェル名の前に `-' を付加し、シェルに ログイン時のスタートアップファイルを読ませる。

コンパイル時のデフォルトのPATH+シェルのログイン時のスタートアップファイルになる。

コンパイル時のデフォルトはutil-linux(centos7のsuを含む)のソースコードを見ると

// util-linux-2.23.2/login-utils/su-common.c 
  if (pw->pw_uid)
    r = logindefs_setenv("PATH", "ENV_PATH", _PATH_DEFPATH);

  else if ((r = logindefs_setenv("PATH", "ENV_ROOTPATH", NULL)) != 0)
    r = logindefs_setenv("PATH", "ENV_SUPATH", _PATH_DEFPATH_ROOT);

となっていて

// util-linux-2.23.2/include/pathnames.h
#undef _PATH_DEFPATH
#define _PATH_DEFPATH           "/usr/local/bin:/bin:/usr/bin"

#undef _PATH_DEFPATH_ROOT
#define _PATH_DEFPATH_ROOT      "/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin"

となっているので

/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin

さらに、/root/.bash_profileに

PATH=$PATH:$HOME/bin

が書かれているので/root/binが追加されている。

sudoした場合はそもそもデフォルトでsecure_pathの設定がしているので、

# /etc/sudoers
Defaults    secure_path = /sbin:/bin:/usr/sbin:/usr/bin

上記のパスがベースになり、シェルを普通に実行する場合(-s)とログインシェルとして実行する場合(-i)との違いがある。

ログインシェルだと/etc/profileも読み込まれるので

# /etc/profile
# Path manipulation
if [ "$EUID" = "0" ]; then
    pathmunge /usr/sbin
    pathmunge /usr/local/sbin
else
    pathmunge /usr/local/sbin after
    pathmunge /usr/sbin after
fi

が実行されて/usr/local/sbinが付加される。

PostgreSQLのbyteaカラムが\xXXXXXXと表示される

$ psql test
test=> create table t(b bytea);
CREATE TABLE
test=> insert into t(b) values('foo');
INSERT 0 1
test=> select * from t;
    b     
----------
 \x666f6f
(1 行)

'foo'を挿入すると'\x666f6f'と16進表示される。

PostgreSQL 9.0の仕様変更っぽい。

byteaの出力はデフォルトで16進数書式になりました。(Peter Eisentraut)

互換性が必要ならばbytea_outputサーバパラメータを使用して伝統的な出力書式を選択することができます。

リリース9.0

これを回避するためにはencode関数を利用するか

test=> select encode(b, 'escape') from t;
 encode 
--------
 foo
(1 行)

サーバの設定(postgresql.conf)を変更する

bytea_output (enum)
bytea型の値の出力形式を設定します。 有効な値はhex(デフォルト)、およびescape(PostgreSQLの伝統的な書式)です。 より詳細は項8.4を参照してください。 bytea型は常にこの設定に係わらず、入力時に双方の書式を受け付けます。

クライアント接続デフォルト
bytea_output = 'escape'