読者です 読者をやめる 読者になる 読者になる

てきとうなメモ

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

Apache Commons FileUploadの脆弱性(CVE-2016-3092)

Security

JVNDB-2016-000121 - Apache Commons FileUpload におけるサービス運用妨害 (DoS) の脆弱性

Apache Commons FileUploadにDoS攻撃脆弱性なのだが、攻撃方法の詳細は説明されていない。調べたところ、推測なのだが、2年前のDoS攻撃脆弱性(CVE-2014-0050)と関係するものではないかと思う。

2年前の脆弱性はこちら。

JVN#14876762: Apache Commons FileUpload におけるサービス運用妨害 (DoS) の脆弱性

この脆弱性はboundaryに長い文字列を指定してマルチパートのPOSTリクエストを送ると発生し、無限ループに陥ることがあるというものである。

この脆弱性に対するPoCはネットを探せば見つかる。どうも以下の2つの条件を満たすと攻撃できるようだ

  • boundaryを4092バイト以上にする
  • bodyに4097バイト以上のboundaryを含まないデータを入れる

適当にサーブレットを作ってPoCを実行すると無限ループに陥る。サーブレットを停止してスタックトレースを見ると以下でループしているようだ。
commons uploadのバージョンは1.3を用いた

MultipartStream$ItemInputStream.makeAvailable() line: 1016    
MultipartStream$ItemInputStream.read(byte[], int, int) line: 901    
MultipartStream$ItemInputStream(InputStream).read(byte[]) line: 101    
Streams.copy(InputStream, OutputStream, boolean, byte[]) line: 101    
Streams.copy(InputStream, OutputStream, boolean) line: 70    
MultipartStream.readBodyData(OutputStream) line: 589    
MultipartStream.discardBodyData() line: 613    
MultipartStream.skipPreamble() line: 630    
FileUploadBase$FileItemIteratorImpl.findNextItem() line: 1018   
FileUploadBase$FileItemIteratorImpl.<init>(FileUploadBase, RequestContext) line: 998    
DiskFileUpload(FileUploadBase).getItemIterator(RequestContext) line: 310    
DiskFileUpload(FileUploadBase).parseRequest(RequestContext) line: 334   
DiskFileUpload(FileUploadBase).parseRequest(HttpServletRequest) line: 288 

ループしているのはMultipartStream$ItemInputStream.makeAvailableだが、このメソッドを説明するためには各クラスの説明を先にした方が良い。
関係する主要なクラスはMultipartStreamとItemInputStreamである。

MultipartStreamはマルチパートのPOSTリクエストをparseするクラスであり、子クラスItemInputStreamはboundaryで分割された現在のitemを読みだすためのクラスである。

この2つのクラスは効率化のために内部のバッファを利用している。このバッファの処理が脆弱性の原因である。

まず、MultipartStreamはバッファ関係のフィールドとして以下のものを持っている。

byte[] buffer バッファを入れるバイト配列
int bufSize bufferのサイズ
int head buffer上の実際のバッファの開始インデックス
int tail buffer上の実際のバッファの終了インデックス
byte[] boundary boundary+prefixを入れておくバイト配列
int boundaryLength boundary.length
int keepRegion boundaryをバッファから探すために必要なサイズ

さらに、ItemInputStream側にもバッファ関係のフィールドとして以下のものがある。

int pos boundaryを検索した時の、boundaryのMultipartStream.buffer上の開始インデックス
int pad boundaryを検索するために保持すべき実際のバイト数。MultipartStream.keepRegionと似たようなもの

headとtailはbuffer内の有効な部分を指している。1バイト読みだすとheadをインクリメントする。head=tailになるとInputStreamから実際に読み出してbufferに入れている。

boundaryはリクエストで指定されたboundaryを入れるバイト列だが、それだけでなくprefixに[CR,LF,-,-]がくっついている。これは実際の区切り文字にはboundaryに--がくっつくためであり、さらに行頭なのでCR,LFがprefixとしてくっついている。この辺りはコンストラクタのコードを見ることで確認できる。

MultipartStream(InputStream input,
      byte[] boundary,
      int bufSize,
      ProgressNotifier pNotifier) {
  ...
  // BOUNDARY_PREFIXは[CR, LF, -, -]
  this.boundary = new byte[boundary.length + BOUNDARY_PREFIX.length];
  this.boundaryLength = boundary.length + BOUNDARY_PREFIX.length;
  this.keepRegion = this.boundary.length;
  System.arraycopy(BOUNDARY_PREFIX, 0, this.boundary, 0,
          BOUNDARY_PREFIX.length);
  System.arraycopy(boundary, 0, this.boundary, BOUNDARY_PREFIX.length,
          boundary.length);
  ...
}

MultipartStream内では、buffer内からboundaryを検索する際にboundaryの途中までマッチしたという情報を残していない。そのままだと読み込んだ単位の境にboundaryがある場合は検知できない。そのため、バッファを全て読み込んでしまい入力ストリームからバッファに読み込む時に、boundaryのサイズだけは常にバッファに残しておき、残りの部分に入力ストリームからデータを書き込む必要性がある。このboundaryの検索のために保持すべきサイズがkeepRegionであり、padは

pad = min(tail-head, keepRegion)

である。

ItemInputStream.makeAvailableの話に戻る。makeAvailableは読み込み可能なデータがバッファ内にない場合にデータを入力ストリームから読み込みバッファに入れて、読み込み可能なバイト数を返すメソッドである。

private int makeAvailable() throws IOException {
    // boundaryが見つかったので終了
    if (pos != -1) {
        return 0;
    }   

    // head〜tailのうち後ろ側のpad分をbufferの最初にもってくる
    // Move the data to the beginning of the buffer.
    total += tail - head - pad;
    System.arraycopy(buffer, tail - pad, buffer, 0, pad);

    // 有効なのは0〜padになる
    // Refill buffer with new data.
    head = 0;
    tail = pad;

    for (;;) {
        // bufferの有効ではない部分に入力から読み込む
        int bytesRead = input.read(buffer, tail, bufSize - tail);
        if (bytesRead == -1) {
            // The last pad amount is left in the buffer.
            // Boundary can't be in there so signal an error
            // condition.
            final String msg = "Stream ended unexpectedly";
            throw new MalformedStreamException(msg);
        }   
        if (notifier != null) {
            notifier.noteBytesRead(bytesRead);
        }   

        // tailを読んだ分だけ拡張する
        tail += bytesRead;

        // buffer内からboundaryを検索し、その位置をposに入れる
        // boundaryが見つからなければ、pos=-1になる
        findSeparator();
        // bufferから読み込み可能なデータを探して、読み込み可能なバイト数を返す
        int av = available();

        // 読み込み可能なデータがあるか、boundaryが見つかった場合はバイト数を返す
        if (av > 0 || pos != -1) {
            return av; 
        }   
    }   
}

available()は現在の読み込み可能なバイト数を返すが、ちょっと特殊で以下のようになっている。

public int available() throws IOException {
  if (pos == -1) {
    return tail - head - pad;
  }
  return pos - head;
}

buffer内にboundaryが見つかった場合はboundaryの開始インデックスまでのサイズを返すが、見つからなければ今有効なサイズ(tail-head)からboundaryに必要なサイズを除いたサイズを返している。boundaryのサイズ分はすぐには使えないと考えているようだ。

で、ループしているのはmakeAvailableのfor文の部分である。

このforループが以下のような条件で攻撃された時の動作を考えてみる。

  • boundaryを4092文字の文字列とし、
  • 入力ストリームがboundaryのない4097文字のバイト列とする。

初期状態は以下のようになっている

  • head=tail=0
  • bufferは用意されているが、0 fillされている
  • bufSize=4096(デフォルト)
  • boundaryにはCR,LF,-,-,boundaryの値が入っている。よって4096文字
  • keepRegionはboundary.lengthなので4096

すると、以下のように処理が進む

  1. 初期状態ではhead=tail=0となっている。
  2. input.readでバッファを読んでいき、tailも読んだサイズに拡張される。
  3. 初回でバッファのサイズ文読み込まれるとは限らないが、バッファサイズまで読み込まれない場合でも、boundaryが見つからずpos=-1になり、available()もtail-head-pad=tail-head-min(tail-head, 4096)=tail-head-(tail-head)=0を返すので、次のループでバッファ全体に読み込まれる
  4. バッファ全体を読み込んだ時には以下のようになり、ループ条件を抜けられない
    • head = 0, tail = 4096
    • pad = min(tail-head, 4096) = 4096
    • pos = -1 (boundaryは発見できない)
    • available() = tail-head-pad = 0
  5. その後も上記の状態のままinput.readしても0バイト読み込むだけなので終了しない

となって無限ループする。

commons fileuploadのv1.3.1ではこの問題が修正されており、以下のコードが追加されている。

@@ -331,9 +326,14 @@
 
         // We prepend CR/LF to the boundary to chop trailing CR/LF from
         // body-data tokens.
-        this.boundary = new byte[boundary.length + BOUNDARY_PREFIX.length];
         this.boundaryLength = boundary.length + BOUNDARY_PREFIX.length;
+        if (bufSize < this.boundaryLength + 1) {
+            throw new IllegalArgumentException(
+                    "The buffer size specified for the MultipartStream is too small");
+        }
+        this.boundary = new byte[this.boundaryLength];
         this.keepRegion = this.boundary.length;
+
         System.arraycopy(BOUNDARY_PREFIX, 0, this.boundary, 0,
                 BOUNDARY_PREFIX.length);
         System.arraycopy(boundary, 0, this.boundary, BOUNDARY_PREFIX.length,

boundaryLengthはリクエスト指定したboundaryのサイズ+4なので4092バイト以上のboundaryが指定された場合はエラーになるようになった。

さて、大元のCVE-2016-3092の話に戻る。boundaryが4092バイト以上の場合は良いが4091バイトだったらどうか。
バッファいっぱいまで読み込んだ状態では

  • head=0,tail=4096
  • keepRegion=4091+4=4095
  • pad=min(tail-head, keepRegion)=min(4096,4095)=1

となり、availableは1を返す。ではmakeAvailableを読んでいる方は以下のコードになる

public int read(byte[] b, int off, int len) throws IOException {
  if (closed) {
    throw new FileItemStream.ItemSkippedException();
  }
  if (len == 0) {
    return 0;
  }
  int res = available();
  if (res == 0) {
    // resに1が入る
    res = makeAvailable();
    if (res == 0) {
      return -1;
    }
  }
  res = Math.min(res, len);
  // 1バイトしかコピーされない
  System.arraycopy(buffer, head, b, off, res);
  head += res;
  total += res;
  return res;
}

1バイトに対するSystem.arraycopyが発生する。リクエストボディが小さければ大したことないが1MB程度のリクエストを送るとこれを1バイトずつ処理して100万回近くループが発生してしまう。おそらくこれがCVE-2016-3092の脆弱性ではないかと思う。

This caused the file
upload process to take several orders of magnitude longer than if the
boundary length was the typical tens of bytes.

http://mail-archives.apache.org/mod_mbox/www-announce/201606.mbox/%3C6223ece6-2b41-ef4f-22f9-d3481e492832@apache.org%3E

と無限ループになるわけではないし、今回の修正のdiffをみるとコンストラクタで最低でもboundaryLengthの2倍をとるようになっているという点とも合致する。

@@ -325,12 +325,6 @@
         if (boundary == null) {
             throw new IllegalArgumentException("boundary may not be null");
         }
-
-        this.input = input;
-        this.bufSize = bufSize;
-        this.buffer = new byte[bufSize];
-        this.notifier = pNotifier;
-
         // We prepend CR/LF to the boundary to chop trailing CR/LF from
         // body-data tokens.
         this.boundaryLength = boundary.length + BOUNDARY_PREFIX.length;
@@ -338,6 +332,12 @@
             throw new IllegalArgumentException(
                     "The buffer size specified for the MultipartStream is too small");
         }
+
+        this.input = input;
+        this.bufSize = Math.max(bufSize, boundaryLength * 2);
+        this.buffer = new byte[this.bufSize];
+        this.notifier = pNotifier;
+
         this.boundary = new byte[this.boundaryLength];
         this.keepRegion = this.boundary.length;

こうすることで、バッファの半分程度はavailable()で返るようになり、デフォルトのバッファのサイズが4KBなので最低でも2KB程度ずつは処理されることになる。