GitLabコードベースにおけるシェルコマンドのガイドライン

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

リファレンス

シェルコマンドの代わりにFileとFileUtilsを使用します。

基本的なUnixコマンドをシェルから呼び出すことがありますが、そのためのRuby APIもあります。 Ruby APIがあれば、それを使ってください。http://www.ruby-doc.org/stdlib-2.0.0/libdoc/fileutils/rdoc/FileUtils.html#module-FileUtils-label-Module+Functions 

# 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 を防ぐことができました。

オプションと引数は – で区切ります。

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

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

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

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

上の例では、cat の引数パーサーは、-l がオプションであると仮定しています。上の例での解決策は、cat に対して、-l が本当はオプションではなく引数であることを明確にすることです。多くの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 を防ぐことができました。

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

シェルコマンドの出力をバックスティックでキャプチャするのはうまく読み取れますが、コマンドを一つの文字列としてシェルに渡さざるを得ません。 これが安全でないことは上で説明しました。 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 のような他のリポジトリでは、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')

重要なのは、|で始まる名前の「ファイル」を開くことです。影響を受けるメソッドには、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と解釈し、リポジトリを新しいプロジェクトにインポートしてしまいます。このアクションは、非公開かどうかにかかわらず、攻撃者にシステム内のあらゆるリポジトリへのアクセスを許してしまう可能性があります。