要約
- 「ユーザー入力」でのファイル名生成が、そもそも非推奨
- 正規表現での、バリデーションチェックでは不十分
- 静的解析ツールは設計レベルを見ている
- Replaceのような細かい実装も注意が必要
背景
ASP Classic / VBScript の既存システム
これらをセキュアコーディングする際に引っかかった内容になります。
正確には、CSVダウンロード改修にて
動的なファイル名出力を行う際エスケープ処理で対応した場合
にひっかかりました。
動的な命名箇所
CSVダウンロード機能を改修していた際、
ユーザーが任意にファイル名を指定できる仕様でした。
Dim fileName
fileName = Request.Form("fileName")
Request.Formから受け取った値をファイル操作に使うと
Path Traversalのリスクがあります
そもそも、Path Traversalとは
../
../ を利用して上位ディレクトリへ移動し、
本来アクセスできないディレクトリへ移動する攻撃です。
/download?file=../../web.config
例えば、
ユーザー入力で動的にファイル名を作る場合上記のような入力が通ると
- 設定ファイル
- DB接続情報
- システム内部ファイル
などが漏洩する可能性があります。
最初の対策：正規表現によるバリデーション
Function CheckFileName(strFileName, strExt)
Dim reg, strBaseName
Set reg = New RegExp
reg.global = true
reg.Pattern = "^[a-zA-Z0-9_]+$"
' 拡張子を除去
strBaseName = Replace(strFileName, strExt, "")
If (reg.Test(strBaseName)) Then
CheckFileName = strBaseName & strExt
Else
Response.Write "Invalid file name"
CheckFileName = ""
End If
End Function
..//.\
この内容で、上記は弾けるため十分と思い
解析ツールに回した結果NGを受けました。
なぜNGなのか？
Request.Form → ファイル操作
というデータフローそのものが危険と判定されていたためです。
静的解析ツールの観点では、
- 今は安全でも
- 将来コード変更される可能性がある
- チェック漏れが発生するかもしれない
という前提で判定します。
つまり、
ユーザー入力をファイル処理に使っている
という設計自体がリスク扱いになります。
セキュアコーディングの方向性
危険な入力を使わない
というのが前提にあるため、ツールで引っかかる内容になりました。
Replaceの使い方にも注意点が
今回のバリデーションで期待していたことは、
拡張子を除いた部分が安全かのチェックでした。
→除去した結果として、安全化の判定を行ってました。
strBaseName = Replace(strFileName, strExt, "")
Replace は一致した文字列をすべて削除します。
test.csv.csv
そのため、上記のような入力に対して、
意図しない除去が発生する可能性があります。
正しい拡張子除去方法
末尾だけを安全に除去するなら、こちらの方が安全になります。
If Right(strFileName, Len(strExt)) = strExt Then
strBaseName = Left(strFileName, Len(strFileName) - Len(strExt))
End If
最終的な解決策
ユーザー入力を使わず、下記で対処するという結論に至りました。
- サーバー側で安全なファイル名を生成
- Sessionに保存
- CSV出力時はSessionから取得
fileName = Session("CsvFileName")