Gormにおける「仕様通り」なSQLインジェクションの恐れのある実装についての注意喚起

Golang

ANDPADボードチームの原田(tomtwinkle)です。

Node.jsの mysqljs/mysql の仕様に起因するSQLインジェクションが話題に上がっていたので、それGolangのORMであるGormでも同じような「仕様」があるよ！ という注意喚起の意味も込めて筆を執りました。

※ 2022/02/21追記 コードレビューを自動化して指摘してもらう記事を公開しました！

tech.andpad.co.jp

Node.jsのMySQLパッケージにおけるエスケープ処理だけでは防げない「隠れた」SQLインジェクション | 株式会社Flatt Security

TL;DR

GormのConditions関数(Find, First, Delete...)を使用する際、第2引数の値にStringを引き渡す場合、idの検索にならずに渡したstringそのものがwhereの条件として引き渡される仕様があります。 なるべくWhere Conditions関数を用いてQueryを構築しましょう。

   var users []User

    userInputID := "1"
    // OK: SELECT * FROM `users`  WHERE (`users`.`id` = '1')
    if err := db.Debug().Find(&users, userInputID).Error; err != nil {
        fmt.Println(err.Error())
        return
    }

    userInputID := "1=1"
    // NG: SELECT * FROM `users`  WHERE (1=1)
    if err := db.Debug().Find(&users, userInputID).Error; err != nil {
        fmt.Println(err.Error())
        return
    }

GormのQuery Conditions関数に関する危険な仕様

Gormはstructをreflectionを用いて解析し、動的にSQLを発行してくれる非常に多機能なORMです。 しかし、多機能である反面実装者がGormのことを熟知していないと危険な動作をする可能性があります。 今回はその中でも意識しないで使用していると脆弱性につながる Conditions関数の第2引数にparameterでstringが引き渡されている パターンの解説をします。

Gorm のQuery Conditionsはかなり多機能で人によって様々な書き方ができるように設計されています。

db.Where("id = ?", "1").Find(&users)
// SELECT * FROM `users`  WHERE (id = '1')
db.Where(User{ID: 1}).Find(&users)
// SELECT * FROM `users`  WHERE (`users`.`id` = 1)
db.Find(&users, "id = ?", "1")
// SELECT * FROM `users`  WHERE (id = '1')
db.Find(&users, User{ID: 1})
//  SELECT * FROM `users`  WHERE (`users`.`id` = 1)
db.Find(&users, "1")
// SELECT * FROM `users`  WHERE (`users`.`id` = '1')

一見全てORMの内部でSQLが組み立てられprepared statementを利用してSQLが発行されているのだろうと想像出来ます……大体はその通りなのですが実は一番最後のパターンにトラップがあります。

具体的には、例えばクライアントから以下のようなパラメーターが渡されてきたとします。

userInputID := "1=1"
db.Find(&users, userInputID)

想定される発行SQLとしては

SELECT * FROM `users`  WHERE (`users`.`id` = '1=1')

つまりIDの値としては存在しない値なので何も取得できないだろうと予想できますが、実際に発行されるSQLは

SELECT * FROM `users`  WHERE 1=1

となり、意図していない値まで全件取得できてしまいます。

更に仮にMySQLの接続時に mysql://tcp(127.0.0.1:3306)/myDB?multiStatements=true のようにmultiStatements=true を指定してセミコロン区切りでの複数SQL発行を許可していた場合は……

userInputID := "1=1);DROP table users;--"
db.Find(&users, userInputID)
// SELECT * FROM `users`  WHERE (1=1);DROP table users;--)

users テーブルが削除されてしまいました。 他のテーブルの取得や更新や削除など好きなSQLが自由に発行できるお祭り状態になります。

対策

根本的な対策としてはFind() や First() Last() Take() Delete()などの関数ではmappingするstructを引数に渡すだけにしてQuery Conditionsでは必ず Where() を使用して条件を指定することです。

db.Where("id = ?", 1).Find(&users)

ANDPADボードでは上記のような危険な実装を実装者が行った場合に returntocorp/semgrep とreviewdog/reviewdog を使用してPull Requestの段階で警告を出すようにしています。

締め

今回説明したもの以外にも structのFieldにzero値を設定するとWhere Statementが生成されない

db.Where(User{ID: 0}).Find(&users)
// SELECT * FROM `users`

など危険な挙動を行うGormですが公式見解では仕様通りとなっており「Security」ページを用意して警告しています。

https://gorm.io/docs/security.html

Libraryを使用する際はそのLibraryの公式ドキュメントやGithub issue等を熟読してから使いましょう。

今回のコードは以下にまとめました。

https://github.com/tomtwinkle/vulnerability-verification-go-gorm

アンドパッドでは一緒に働く仲間を大募集しています。 社内gopherコミュニティが活発に活動しております。 ご興味を持たれた方はぜひカジュアル面談や情報交換のご連絡ください！

engineer.andpad.co.jp