[インデックス 14376] ファイルの概要

このコミットは、Go言語の標準ライブラリ archive/zip パッケージにおける、ZIPファイルのExtraフィールドのパース処理に関するバグ修正です。具体的には、Extraフィールドが途中で切り詰められている（truncated）場合に発生するパニック（bounds check panic）を修正し、より堅牢なエラーハンドリングを導入しています。

コミット

commit 20a181583376b6f0027f73e3c80733c252fecceb
Author: David McLeish <davemc@google.com>
Date:   Mon Nov 12 12:21:00 2012 +0100

    archive/zip: Fix bounds check panic for ZIP files with a truncated extra header.
    
    R=adg, dave
    CC=gobot, golang-dev
    https://golang.org/cl/6811080

GitHub上でのコミットページへのリンク

https://github.com/golang/go/commit/20a181583376b6f0027f73e3c80733c252fecceb

元コミット内容

archive/zip: Fix bounds check panic for ZIP files with a truncated extra header.

このコミットは、ZIPファイルが切り詰められたExtraヘッダーを持っている場合に発生する境界チェックパニックを修正します。

この変更は、Go言語のarchive/zipパッケージが、不正な形式のZIPファイル、特にExtraフィールドが不完全に記述されているファイルに対して脆弱であった問題に対処しています。元の実装では、Extraフィールドのパース時に、期待されるデータ長が実際のバッファサイズを超えた場合に、Goランタイムが提供する境界チェックによってパニックが発生する可能性がありました。これは、悪意のある、または破損したZIPファイルによってサービス拒否（DoS）攻撃を引き起こす可能性を秘めていました。

この問題は、GoのIssue 4302として報告されており、TestInvalidExtraHedaerというテストケースが追加されています。このテストは、不正なExtraヘッダーを持つZIPファイルを意図的に作成し、archive/zipパッケージがErrFormatを返すことを期待しています。しかし、修正前はパニックが発生していました。

前提知識の解説

ZIPファイルフォーマット

ZIPファイルは、複数のファイルやディレクトリを単一のアーカイブにまとめるための一般的なファイルフォーマットです。その構造は、主に以下の要素で構成されます。

ローカルファイルヘッダー (Local File Header): 各ファイルのエントリの先頭に位置し、ファイル名、圧縮方法、圧縮・非圧縮サイズなどの情報を含みます。
ファイルデータ (File Data): 圧縮された実際のファイルデータです。
データ記述子 (Data Descriptor): ローカルファイルヘッダーに圧縮・非圧縮サイズが含まれていない場合に使用されます。
セントラルディレクトリヘッダー (Central Directory Header): ZIPアーカイブ内のすべてのファイルエントリのメタデータ（ファイル名、圧縮方法、ファイルサイズ、ローカルファイルヘッダーへのオフセットなど）を集中管理します。これにより、アーカイブ全体を効率的にスキャンできます。
セントラルディレクトリの終端レコード (End of Central Directory Record): セントラルディレクトリの開始位置やアーカイブ内のエントリ数などの情報を含み、ZIPファイルの末尾に位置します。

Extraフィールド

ZIPファイルフォーマットでは、ローカルファイルヘッダーとセントラルディレクトリヘッダーの両方に「Extraフィールド」と呼ばれる可変長の領域を設けることができます。このフィールドは、標準のZIPフォーマットでは定義されていない追加のメタデータ（例: ZIP64拡張情報、UNIXタイムスタンプ、NTFS属性など）を格納するために使用されます。

Extraフィールドは、通常、以下の構造で構成されます。

ヘッダーID (Header ID): 2バイト。Extraフィールドのタイプを識別するID。
データサイズ (Data Size): 2バイト。ヘッダーIDに続くデータのバイト長。
データ (Data): Data Sizeで指定された長さのデータ。

複数のExtraフィールドが連結されることもあります。

Go言語の`archive/zip`パッケージ

Go言語の標準ライブラリには、ZIPアーカイブの読み書きをサポートするarchive/zipパッケージが含まれています。このパッケージは、ZIPファイルの構造を抽象化し、Goプログラムから簡単にZIPファイルを操作できるようにします。

zip.Reader: ZIPファイルを読み込むための構造体。
zip.File: ZIPアーカイブ内の個々のファイルエントリを表す構造体。Extraフィールドもこの構造体に含まれます。
readDirectoryHeader: セントラルディレクトリヘッダーを読み込む内部関数。この関数内でExtraフィールドのパースが行われます。
readBuf: バイトスライスからデータを読み込むためのヘルパー型。uint16()などのメソッドを提供します。
ErrFormat: ZIPファイルのフォーマットが不正な場合に返されるエラー。

境界チェックパニック (Bounds Check Panic)

Go言語は、メモリ安全性を高めるために、スライスや配列へのアクセス時に自動的に境界チェックを行います。これは、インデックスがスライスの範囲外である場合にランタイムパニック（"index out of range"）を引き起こすことで、不正なメモリアクセスを防ぐ仕組みです。

今回の問題は、Extraフィールドのパースにおいて、Data Sizeで指定された長さが、実際に利用可能なバッファの長さを超えていた場合に、この境界チェックがトリガーされてパニックが発生するというものでした。

技術的詳細

このコミットの技術的な核心は、src/pkg/archive/zip/reader.goファイルのreadDirectoryHeader関数におけるExtraフィールドのパースロジックの改善にあります。

修正前は、Extraフィールドのパースループの条件がlen(b) > 0でした。これは、b（Extraフィールドのバイトスライス）にデータが残っている限りループを続けることを意味します。ループ内で、tag（ヘッダーID）とsize（データサイズ）をそれぞれ2バイトずつ読み込みます。問題は、sizeで指定された長さが、残りのbの長さよりも大きい場合に発生しました。この場合、b = b[size:]というスライス操作が、sizeがlen(b)を超えているため、境界チェックパニックを引き起こしていました。

修正後のコードでは、以下の2つの重要な変更が加えられています。

ループ条件の変更: for len(b) > 0 から for len(b) > 4 に変更されました。
- Extraフィールドの各エントリは、最低でも2バイトのtagと2バイトのsize、合計4バイトを必要とします。
- len(b) <= 4 の場合、tagとsizeを安全に読み込むための十分なデータがないことを意味します。この条件により、不完全なExtraフィールドエントリのパースを試みる前にループを終了させることができます。
- これにより、b.uint16()（tagとsizeの読み込み）が境界外アクセスを引き起こす可能性が低減されます。
残りのデータのチェック: ループの後に if len(b) != 0 { return ErrFormat } が追加されました。
- これは、Extraフィールドのパースが完了した後、b（Extraフィールドの残りのバイトスライス）が空であるべきだという前提に基づいています。
- もしbが空でない場合、それはExtraフィールドが不完全に終了している（例えば、最後のExtraフィールドエントリが切り詰められている）ことを意味します。
- このような不正なフォーマットのZIPファイルに対しては、パニックを引き起こすのではなく、ErrFormatエラーを返すことで、より堅牢なエラーハンドリングを実現しています。

これらの変更により、archive/zipパッケージは、不正な形式のExtraフィールドを持つZIPファイルに対しても、パニックすることなく、適切なエラー（ErrFormat）を返すようになりました。

コアとなるコードの変更箇所

src/pkg/archive/zip/reader.go

--- a/src/pkg/archive/zip/reader.go
+++ b/src/pkg/archive/zip/reader.go
@@ -238,7 +238,7 @@ func readDirectoryHeader(f *File, r io.Reader) error {
 
  	if len(f.Extra) > 0 {
  		b := readBuf(f.Extra)
- 		for len(b) > 0 {
+ 		for len(b) > 4 { // need at least tag and size
  			tag := b.uint16()
  			size := b.uint16()
  			if int(size) > len(b) {
@@ -259,6 +259,10 @@ func readDirectoryHeader(f *File, r io.Reader) error {
  			}
  			b = b[size:]
  		}
+ 		// Should have consumed the whole header.
+ 		if len(b) != 0 {
+ 			return ErrFormat
+ 		}
  	}
  	return nil
  }

src/pkg/archive/zip/zip_test.go

--- a/src/pkg/archive/zip/zip_test.go
+++ b/src/pkg/archive/zip/zip_test.go
@@ -174,13 +174,31 @@ func TestZip64(t *testing.T) {
  	}
  }
 
-// Issue 4302.
-func TestInvalidExtraHedaer(t *testing.T) {
-	const timeFormat = "20060102T150405.000.txt"
-
+func testInvalidHeader(h *FileHeader, t *testing.T) {
 	var buf bytes.Buffer
 	z := NewWriter(&buf)
 
+	f, err := z.CreateHeader(h)
+	if err != nil {
+		t.Fatalf("error creating header: %v", err)
+	}
+	if _, err := f.Write([]byte("hi")); err != nil {
+		t.Fatalf("error writing content: %v", err)
+	}
+	if err := z.Close(); err != nil {
+		t.Fatal("error closing zip writer: %v", err)
+	}
+
+	b := buf.Bytes()
+	if _, err = NewReader(bytes.NewReader(b), int64(len(b))); err != ErrFormat {
+		t.Fatal("got %v, expected ErrFormat", err)
+	}
+}
+
+// Issue 4302.
+func TestHeaderInvalidTagAndSize(t *testing.T) {
+	const timeFormat = "20060102T150405.000.txt"
+
 	ts := time.Now()
 	filename := ts.Format(timeFormat)
 
@@ -191,19 +209,14 @@ func TestInvalidExtraHedaer(t *testing.T) {
 	}
 	h.SetModTime(ts)
 
-	fh, err := z.CreateHeader(&h)
-	if err != nil {
-		t.Fatalf("error creating header: %v", err)
-	}
-	if _, err := fh.Write([]byte("hi")); err != nil {
-		t.Fatalf("error writing content: %v", err)
-	}
-	if err := z.Close(); err != nil {
-		t.Fatal("error closing zip writer: %v", err)
-	}\n
-	b := buf.Bytes()
-	if _, err = NewReader(bytes.NewReader(b), int64(len(b))); err == nil {
-		t.Fatal("expected ErrFormat")
+	testInvalidHeader(&h, t)
+}
+
+func TestHeaderTooShort(t *testing.T) {
+	h := FileHeader{
+		Name:   "foo.txt",
+		Method: Deflate,
+		Extra:  []byte{zip64ExtraId}, // missing size
 	}
+	testInvalidHeader(&h, t)
 }

コアとなるコードの解説

`src/pkg/archive/zip/reader.go` の変更点

for len(b) > 4:
- この変更は、Extraフィールドの各サブエントリが最低でも4バイト（2バイトのtagと2バイトのsize）を必要とすることを考慮しています。
- もしbの残りの長さが4バイト以下であれば、完全なtagとsizeのペアを読み込むことができないため、ループを継続しても意味がありません。これにより、b.uint16()が境界外アクセスを引き起こす可能性を未然に防ぎます。
if len(b) != 0 { return ErrFormat }:
- ループが終了した後、bがまだ空でない場合、それはExtraフィールドのデータが不完全に終了していることを意味します。
- 例えば、Extraフィールドの最後に1バイトだけ残っているような場合、これは不正なフォーマットです。
- このようなケースでパニックする代わりに、ErrFormatエラーを返すことで、呼び出し元が適切にエラーを処理できるようにします。これは、不正な入力に対する堅牢性を高めるための重要な変更です。

`src/pkg/archive/zip/zip_test.go` の変更点

testInvalidHeader 関数の導入:
- 複数の不正なヘッダーテストケースで共通のロジックを再利用するために、testInvalidHeaderというヘルパー関数が導入されました。
- この関数は、与えられたFileHeaderを使用してZIPファイルを作成し、そのZIPファイルを読み込もうとしたときにErrFormatが返されることを検証します。
TestHeaderInvalidTagAndSize:
- 元のTestInvalidExtraHedaerがTestHeaderInvalidTagAndSizeにリネームされました。
- このテストは、Extraフィールドに不正なtagやsizeが含まれるケースをテストします。具体的には、Extraフィールドに0x0001（zip64ExtraId）と0x0000（サイズ0）を設定し、その後にさらにデータが続くような不正な構造を意図的に作成しています。
TestHeaderTooShort の追加:
- この新しいテストケースは、Extraフィールドがtagのみでsizeが欠落している、つまりExtraフィールド全体が途中で切り詰められているケースを明示的にテストします。
- Extra: []byte{zip64ExtraId} のように、tagは存在するがsizeが欠落している状況をシミュレートし、この場合もErrFormatが返されることを確認します。

これらのテストの追加と改善により、archive/zipパッケージが様々な不正なExtraフィールドのシナリオに対して正しくエラーハンドリングを行うことが保証されます。

参考にした情報源リンク

ZIPファイルフォーマットの仕様:
- https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT (PKWARE社の公式仕様書)
Go言語のarchive/zipパッケージのドキュメント:
- https://pkg.go.dev/archive/zip
Go言語の境界チェックに関する情報:
- Goのコンパイラとランタイムの最適化に関する一般的な情報源（特定のURLは挙げませんが、Goのパフォーマンスやセキュリティに関する記事で言及されることが多いです）
Go言語のIssueトラッカーとCL (Change List) システム:
- https://go.dev/issue/
- https://go.dev/cl/

comemo