Go 1.19.3 and 1.18.8 security fix の内容を読んだ

今更ながら Go 1.93 及び 1.8.8 のマイナーリリース、security fix の内容について。

リリース内容はこちら

groups.google.com

どういう問題があったか

github.com

Windowsにおいて、環境変数をサブプロセスに渡す際にヌル文字（NUL）が含まれていると別の環境変数を設定できてしまう。例えば “A=B\x00C=D” という文字列を解釈すると環境変数として A=B, C=D と設定できるということ。

修正内容

github.com

diff の上から見ていくと、まず env_test.go は今回の脆弱性がテストケースとして分かりやすく表現されていて良い。

次に os/exec/exec.go が修正されているが、コミットメッセージを見ると、一応冗長なチェックを追加しといた（「Add a redundant check to os/exec as extra insurance.」）と書いてあるため、一旦スルー。

本丸は syscall/exec_windows.go の修正で、createEnvBlock 関数に変更が加わっている。

中身を見る前に、syscall パッケージについておさらいすると、syscall パッケージは OS のシステムコールを担当するパッケージで、syscall パッケージ内にある各 OS 専用のファイルでシステムコールの実装が管理されている。例えば、syscall パッケージの StartProcess 関数の GoDoc には「StartProcess wraps ForkExec for package os.」と書いてある。これは Windows なら exec_windows.go の処理が呼ばれ、UNIX なら exec_unix.go の処理が呼ばれるように、実行環境に合わせた形でビルドされる。ちなみに、Go 1.4 からは syscall パッケージは基本フリーズされ、OS 固有の実装は golang.org/x/sys で管理されている（参考：https://go.googlesource.com/proposal/+/refs/heads/master/design/freeze-syscall.md）。

createdEnvBlock 関数に戻ると、この関数はざっくり言うと環境変数を受け取って、その後の処理に必要な形に変換している。今回の修正では、error を返すようになった。どういうときに error を返すかというと、if bytealg.IndexByteString(s, 0) != -1 { のときに error を返すようになった。

この bytealg パッケージの IndexByteString 関数が何をやってるか調べる。まず bytealg パッケージは internal なパッケージだった。IndexByteString 関数は、pkg.go.dev を見ても特にコメントが無かったため、実装を見にいく。

pkg.go.dev

bytealg.IndexByteString 関数の実装は多分こちら。

go.dev

実装を見ると、文字列とバイトを受け取って、文字列を一文字ずつ for で回してバイトと同じ要素が見つかったらそのインデックスを返し、見つからなかったら -1 を返すということをしている。

(internal パッケージなのでコピペして) playground で動かしてみると、実際にヌル文字がインデックス3つ目にあることを検知できた。

package main

import "fmt"

func main() {
    fmt.Println(IndexByteString("A=B\x00C=D", 0)) // 3
}

func IndexByteString(s string, c byte) int {
    for i := 0; i < len(s); i++ {
        if s[i] == c {
            return i
        }
    }
    return -1
}

よって、Windows において環境変数の文字列を解釈する処理中に、ヌル文字を見つけて error を返すような変更が加わったということが確認できた。