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つの条件を満たすと攻撃できるようだ
適当にサーブレットを作って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
すると、以下のように処理が進む
- 初期状態ではhead=tail=0となっている。
- input.readでバッファを読んでいき、tailも読んだサイズに拡張される。
- 初回でバッファのサイズ文読み込まれるとは限らないが、バッファサイズまで読み込まれない場合でも、boundaryが見つからずpos=-1になり、available()もtail-head-pad=tail-head-min(tail-head, 4096)=tail-head-(tail-head)=0を返すので、次のループでバッファ全体に読み込まれる
- バッファ全体を読み込んだ時には以下のようになり、ループ条件を抜けられない
- head = 0, tail = 4096
- pad = min(tail-head, 4096) = 4096
- pos = -1 (boundaryは発見できない)
- available() = tail-head-pad = 0
- その後も上記の状態のまま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
http://mail-archives.apache.org/mod_mbox/www-announce/201606.mbox/%3C6223ece6-2b41-ef4f-22f9-d3481e492832@apache.org%3E
upload process to take several orders of magnitude longer than if the
boundary length was the typical tens of bytes.
と無限ループになるわけではないし、今回の修正の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程度ずつは処理されることになる。
