シェルコマンド開発ガイドライン

この文書には、GitLabコードベースのプロセスやファイルを扱うためのガイドラインがコンテナされています。これらのガイドラインは、あなたのコードの信頼_性と_セキュリティを高めるためのものです。

リファレンス

シェルコマンドの代わりにFileとFileUtilsを使用してください。

RubyのAPIがあるにもかかわらず、基本的なUnixコマンドをShellで呼び出してしまうことがあります。RubyのAPIがあれば、それを使いましょう。

# Wrong
system "mkdir -p tmp/special/directory"
# Better (separate tokens)
system *%W(mkdir -p tmp/special/directory)
# Best (do not use a shell command)
FileUtils.mkdir_p "tmp/special/directory"

# Wrong
contents = `cat #{filename}`
# Correct
contents = File.read(filename)

# Sometimes a shell command is just the best solution. The example below has no
# user input, and is hard to implement correctly in Ruby: delete all files and
# directories older than 120 minutes under /some/path, but not /some/path
# itself.
Gitlab::Popen.popen(%W(find /some/path -not -path /some/path -mmin +120 -delete))

このコーディングスタイルが CVE-2013-4490 を防げたかもしれません。

Git コマンドには常に設定可能な Git バイナリパスを使うようにしました。 

# Wrong
system(*%W(git branch -d -- #{branch_name}))

# Correct
system(*%W(#{Gitlab.config.git.bin_path} branch -d -- #{branch_name}))

コマンドを個別のトークンに分割してシェルを回避

シェルコマンドを1つの文字列としてRubyに渡すと、Rubyは/bin/sh 、文字列全体を評価します。要するに、1行のスクリプトを評価するようシェルに要求しているのです。これではシェルインジェクション攻撃を受ける危険性があります。シェルコマンドを自分でトークンに分割する方がよいでしょう。シェルのスクリプト機能を使って、作業ディレクトリを変更したり、環境変数を設定したりすることがあります。これらもすべて、Rubyから直接安全に行うことができます。

# Wrong
system "cd /home/git/gitlab && bundle exec rake db:#{something} RAILS_ENV=production"
# Correct
system({'RAILS_ENV' => 'production'}, *%W(bundle exec rake db:#{something}), chdir: '/home/git/gitlab')

# Wrong
system "touch #{myfile}"
# Better
system "touch", myfile
# Best (do not run a shell command at all)
FileUtils.touch myfile

このコーディングスタイルが CVE-2013-4546 を防げたかもしれません。

別の例としてhttps://gitlab.com/gitlab-org/gitlab/-/merge_requests/93030、https://starlabs.sg/blog/2022/07-gitlab-project-import-rce-analysis-cve-2022-2185/ も参照してください。

オプションと引数は – で区切ってください。

-- を使って、システムコマンドの引数パーサーにオプションと引数の違いを明確に伝えます。これは多くのUnixコマンドでサポートされていますが、すべてのUnixコマンドでサポートされているわけではありません。

-- が何をするのかを理解するために、以下の問題を考えてみましょう。

# Example
$ echo hello > -l
$ cat -l

cat: illegal option -- l
usage: cat [-benstuv] [file ...]

上の例では、cat の引数パーサーは、-l がオプションであると仮定しています。上の例の解決策は、-l が本当はオプションではなく引数であることを、cat に明示することです。多くのUnixコマンドラインツールは、オプションと引数を--で区切る慣例に従っています。

# Example (continued)
$ cat -- -l

hello

GitLabのコードベースでは、オプション/引数の曖昧さを避けるために、オプションがサポートされているコマンドでは_常に_ -- 。

# Wrong
system(*%W(#{Gitlab.config.git.bin_path} branch -d #{branch_name}))
# Correct
system(*%W(#{Gitlab.config.git.bin_path} branch -d -- #{branch_name}))

このコーディングスタイルが CVE-2013-4582 を防いでいる可能性があります。

バックティックを使用しないでください。

バックスティックを使ってシェルコマンドの出力をキャプチャすると、うまく読み取れますが、コマンドを1つの文字列としてシェルに渡さざるを得ません。これが安全でないことは上で説明したとおりです。GitLabのコードベースでは、代わりにGitlab::Popen.popen 。

# Wrong
logs = `cd #{repo_dir} && #{Gitlab.config.git.bin_path} log`
# Correct
logs, exit_status = Gitlab::Popen.popen(%W(#{Gitlab.config.git.bin_path} log), repo_dir)

# Wrong
user = `whoami`
# Correct
user, exit_status = Gitlab::Popen.popen(%W(whoami))

GitLab Shellのような他のリポジトリでは、IO.popen を使うこともできます。

# Safe IO.popen example
logs = IO.popen(%W(#{Gitlab.config.git.bin_path} log), chdir: repo_dir) { |p| p.read }

Gitlab::Popen.popen とは異なり、IO.popen は標準エラーをキャプチャしないことに注意してください。

パス文字列の先頭でのユーザー入力の回避

Rubyでファイルを開いたり読んだりするための様々な方法は、ファイルの代わりにプロセスの標準出力を読むために使うことができます。以下の2つのコマンドはだいたい同じことをします：

`touch /tmp/pawned-by-backticks`
File.read('|touch /tmp/pawned-by-file-read')

重要なのは、| で始まる名前の ‘file’ を開くことです。影響を受けるメソッドには、Kernel#open、File::read、File::open、IO::open、IO::read があります。

攻撃者がオープンするファイル名の文字列の先頭を制御できないようにすることで、 「open」と「read」のこの動作から保護することができます。例えば、誤ってシェルコマンドを| ：

# we assume repo_path is not controlled by the attacker (user)
path = File.join(repo_path, user_input)
# path cannot start with '|' now.
File.read(path)

ユーザー入力で相対パスを使用する必要がある場合は、パスの先頭に./ を付けます。

ユーザーが入力したパスのプレフィックスを付けることで、- で始まるパスに対する保護も強化されます (上記の-- の使用に関する議論を参照してください)。

パストラバーサルからのガード

パストラバーサルとは、プログラム(GitLab)がユーザーのアクセスをディスク上の特定のディレクトリに制限しようとしているにもかかわらず、ユーザーが../ パス表記を利用してそのディレクトリ外のファイルを開くことに成功してしまうセキュリティです。

# Suppose the user gave us a path and they are trying to trick us
user_input = '../other-repo.git/other-file'

# We look up the repo path somewhere
repo_path = 'repositories/user-repo.git'

# The intention of the code below is to open a file under repo_path, but
# because the user used '..' they can 'break out' into
# 'repositories/other-repo.git'
full_path = File.join(repo_path, user_input)
File.open(full_path) do # Oops!

これを防ぐ良い方法は、RubyのFile.absolute_path に従ってフルパスとその「絶対パス」を比較することです。

full_path = File.join(repo_path, user_input)
if full_path != File.absolute_path(full_path)
  raise "Invalid path: #{full_path.inspect}"
end

File.open(full_path) do # Etc.

このようなチェックがあれば、CVE-2013-4583を回避できたかもしれません。

文字列の先頭と末尾への正規表現の適切なアンカー付け

シェルコマンドの引数として渡されるユーザー入力を検証するために正規表現を使用する場合、文字列の開始と終了を指定する^ 、$ 、または全くアンカーを使用しないのではなく、\A 、\z アンカーを必ず使用してください。

そうしないと、攻撃者はこれを使って、潜在的に有害な効果を持つコマンドを実行する可能性があります。

例えば、プロジェクトのimport_url が以下のように検証されている場合、ユーザーはGitLabを騙してローカルファイルシステム上のGitリポジトリからクローンすることができます。

validates :import_url, format: { with: URI.regexp(%w(ssh git http https)) }
# URI.regexp(%w(ssh git http https)) roughly evaluates to /(ssh|git|http|https):(something_that_looks_like_a_url)/

ユーザーがインポートの URL として以下を送信したとしましょう：

file://git:/tmp/lol

使用されている正規表現にはアンカーがないので、値のgit:/tmp/lol はマッチし、バリデーションはパスします。

インポートの際、GitLabは引数としてimport_url ：

git clone file://git:/tmp/lol

Git はgit: の部分を無視し、パスをfile:///tmp/lolと解釈し、リポジトリを新しいプロジェクトにインポートします。このアクションは、非公開かどうかにかかわらず、攻撃者にシステム内のどのリポジトリにもアクセスさせる可能性があります。