SQLインジェクションを考慮しつつ、selectするカラムを動的に変更する


## はじめに

この記事では「引数の値によって、selectするカラムを動的に変える実装例」を紹介します。

最初にSQLインジェクションを考慮しないダメな実装例を、次にSQLインジェクションを考慮した実装例を紹介します。

また、Brakeman gemを使った自動検知や、セキュリティ問題に関する情報源についても軽く紹介します。

## やりたいこと

渡された引数に応じて、selectするカラムを動的に変更したい。

## セキュリティ的にダメな実装例

次のように、単純に引数を文字列として埋め込むだけだとNGです。
SQLインジェクションの危険があります。

```ruby
class User < ApplicationRecord
  # ダメな実装例
  scope :dangerous_limited_columns, -> (text) {
    # 引数をそのまま埋め込んだらダメ！！
    select("id, #{text}")
  }
end
```

その理由を以下で説明します。

### SQLインジェクションが発生する例

以下のようなテーブルがあったとします。

```ruby
# schema.rb
ActiveRecord::Schema.define(version: 20170106221656) do

  create_table "users", force: :cascade do |t|
    t.string   "nickname"
    t.string   "twitter"
    t.boolean  "admin",      default: false, null: false
    t.datetime "created_at",                 null: false
    t.datetime "updated_at",                 null: false
  end

end
```

データベースには以下のようなデータが登録されています。
（fixtureとして使用するYAMLファイルで表示）

```yaml
# users.yml
# 一般ユーザー
non_admin:
  nickname: 'じゅんちゃん'
  twitter: '@jnchito'
  admin: false

# システム管理者
admin:
  nickname: 'システム管理者'
  twitter: '@admin'
  admin: true
```

Userクラスには次のようなscopeが定義されています。

```ruby
class User < ApplicationRecord
  # 管理者を除外するscope
  scope :non_admin, -> { where(admin: false) }
  
  # 取得するカラムを制限するscope
  scope :dangerous_limited_columns, -> (text) {
    select("id, #{text}")
  }
end
```

コントローラでは次のようなコードが書いてあったとします。

```ruby
# 管理者（admin）は除外しつつ、取得するカラムを制限する
User.non_admin.dangerous_limited_columns(params['target']).first
```

上のコードを見ればわかるとおり、コントローラでは画面から送られてきたパラメータをそのままscopeの引数に渡しています。

本来であれば`params['target']`には、"nickname"や"twitter"のようなカラム名が入っている想定です。

しかし、画面からはやろうと思えば自由に値をサーバーに送ることができます。
たとえば次のような文字列が送信された場合はどうでしょう？

```sql
nickname, admin FROM users WHERE admin = 't' UNION SELECT id, nickname, admin FROM users WHERE admin = ?  ORDER BY admin DESC LIMIT ? --
```

1行で書いてしまうとわかりづらいので、適当に改行を入れます。

```sql
nickname,
admin
FROM users
WHERE admin = 't'
UNION
SELECT id, nickname, admin
FROM users
WHERE admin = ?
ORDER BY admin DESC
LIMIT ? --
```

このような文字列が`params['target']`に入ってくると、次のようなSQLが実行されます。
（見やすくなるように改行を入れています）

```sql
SELECT id, nickname, admin
FROM users
WHERE admin = 't'
UNION
SELECT id, nickname, admin
FROM users
WHERE admin = ?
ORDER BY admin DESC
LIMIT ?
-- FROM "users" WHERE "users"."admin" = ? ORDER BY "users"."id" ASC LIMIT ?
```

SQLに詳しくない人は何をやっているかぱっとわかりづらいかもしれませんが、これは一言で言うと「システム管理者を先頭に持ってくるSQL」になっています。
なので、本来は表示されるべきでないシステム管理者（admin）のデータも取得できてしまいます。

実際にこの動きをrails console上でシミュレートすると、次のような結果になります。

```ruby
user = User.non_admin.dangerous_limited_columns(<<-SQL.squish).first
  nickname,
  admin
  FROM users
  WHERE admin = 't'
  UNION
  SELECT id, nickname, admin
  FROM users
  WHERE admin = ?
  ORDER BY admin DESC
  LIMIT ? --
SQL
# 実装者の意図に反してシステム管理者のデータが取れてしまう
p user
#=> #<User id: 135138680, nickname: "システム管理者", admin: true>
```

このように、画面の入力値をそのままselectメソッドに埋め込んでしまうと、悪意のあるユーザーが自由にSQLを組んでデータベースのデータを盗み取ってしまう恐れがあります。

## SQLインジェクションを考慮した実装例

というわけで、もし動的に取得するカラムを変更したいのであれば、「何かしらの制限を持たせる方法」を検討する必要があります。

SQLの取得条件（WHERE句）であれば、`where(nickname: 'じゅんちゃん')`や`where("nickname = ?", 'じゅんちゃん')`のように、自動的にサニタイズされるパラメータを使うことができるのですが、selectメソッドの場合はそのようになっていません。
そこで、今回は毎回入力値チェックを行い、有効な引数以外は例外を発生させる実装にしてみます。

```ruby
class User < ApplicationRecord
  scope :non_admin, -> { where(admin: false) }
  
  # 引数として有効なカラムを定義
  ALLOWED_COLUMNS = %w(nickname twitter).freeze
  
  scope :limited_columns, -> (text) {
    # 有効な引数でなければ例外を発生させる
    unless text.in?(ALLOWED_COLUMNS)
      raise ArgumentError, "Invalid argument: #{text}"
    end
    select("id, #{text}")
  }
end
```

このようにすると、不正な入力があったときにSQLの実行を防ぐことができます。

```ruby
user = User.non_admin.limited_columns(<<-SQL.squish).first
  nickname,
  admin
  FROM users
  WHERE admin = 't'
  UNION
  SELECT id, nickname, admin
  FROM users
  WHERE admin = ?
  ORDER BY admin DESC
  LIMIT ? --
SQL
#=> ArgumentError: Invalid argument: nickname FROM users WHERE admin = 't' UNION SELECT id, nickname FROM users WHERE users.admin = ? ORDER BY users.id DESC LIMIT ? --
```

ただし、これはSQLインジェクションを考慮した実装の「一例」にすぎません。
要件によってはこの実装例が採用できなかったりする場合もあると思います。
その場合は何らかの別の方法を検討する必要があります。
SQLに詳しくない人は一人で悩まず、周りの詳しい技術者に相談することをお勧めします。

## Brakeman gemで危険なコードを自動検知する

そもそもRailsやSQLに精通していない人は、どんなコードがSQLインジェクションを発生させるのか、あまり見当が付かないと思います。
もしかすると、自分の知らないうちに他にもSQLインジェクションを発生させるコードを書いてしまっているかもしれません。

そんな場合はツールを使って自動検知するようにすれば、セキュリティ的に問題があるコードを報告してくれます。
RailsではBrakemanというgemが有名です。

[presidentbeef/brakeman: A static analysis security vulnerability scanner for Ruby on Rails applications](https://github.com/presidentbeef/brakeman)

Brakemanを実行すると、以下のように先ほどのコードも警告が発生します。
（注：引数をチェックするパターンも、静的な構文解析の上ではリスクがあると判断されます）

![Screen Shot 2017-01-07 at 8.33.32.png](https://qiita-image-store.s3.amazonaws.com/0/7465/9bd0f985-44f7-40db-2243-1761c3a214b3.png "Screen Shot 2017-01-07 at 8.33.32.png")

ネットを検索すると日本語の情報もいくつかあるので、Brakemanの詳しい使い方についてはそちらを参考にしてください。

## さらに：セキュリティ問題についてちゃんと学ぶ

Webアプリケーションを開発する場合は、セキュリティ問題についてしっかり理解しておく必要があります。
そうしないと、予期しない大問題を引き起こす恐れがあるからです。

RailsにおけるSQLインジェクションについては以下のページで詳しく説明されています。

[Rails SQL Injection Examples（英語）](http://rails-sqli.org/)

SQLインジェクション以外にもセキュリティ問題は存在します。
以下の公式ページはRailsプログラマなら必見です。

[Rails セキュリティガイド \| Rails ガイド](https://railsguides.jp/security.html)

また、書籍では「体系的に学ぶ 安全なWebアプリケーションの作り方」という本が有名です。
この本はサンプルコードとして主にPHPが使われていますが、基本的な考え方はRailsを含むWebアプリケーション全体に有効です。

[体系的に学ぶ 安全なWebアプリケーションの作り方](http://www.amazon.co.jp/dp/B00E5TJ120)

## 別解：selectメソッドに複数の引数を渡す

今回の例で使ったような要件であれば、selectメソッドに複数の引数を渡すようにした方が多少安心です。

```ruby
class User < ApplicationRecord
  # 省略
  ALLOWED_COLUMNS = %w(nickname twitter).freeze
  scope :limited_columns, -> (text) {
    unless text.in?(ALLOWED_COLUMNS)
      raise ArgumentError, "Invalid argument: #{text}"
    end
    # idとtextを別々の引数で渡す
    select(:id, text)
  }
end
```

SQLは次のようになります。

```ruby
User.non_admin.limited_columns('nickname').first
# 以下は実行されるSQL
SELECT  "users"."id", "users"."nickname" 
FROM "users" 
WHERE "users"."admin" = ? 
ORDER BY "users"."id" ASC 
LIMIT ? 
```

selectメソッドに渡した文字列がそのままSQLになるのではなく、Rails側が少し加工してくれるので、そのぶん多少安心かもしれません。（とはいえ、引数をチェックもなしにそのまま渡すのは危険）

## まとめ
というわけで、この記事ではSQLインジェクションを考慮しつつ、selectするカラムを動的に変更する実装例を紹介しました。

そもそもの話になりますが、データ取得時に変数を文字列に直接埋め込む系の実装はSQLインジェクションのリスクを高めがちです。
なので、極力そうした実装を使わないで済む設計を考えることも大事だと思います。

セキュリティ問題を考慮しながら安全なWebアプリケーションを構築するように心がけましょう。

### あわせて読みたい

SQLインジェクションのことが理解出てきていれば、この4コマ漫画のブラックジョーク具合がわかりますw

[隣の同僚がSQLインジェクションを理解しているかどうか一発でわかる4コマ漫画 \- Qiita](http://qiita.com/jnchito/items/4317c08b0348bd1f8c36)