SQLインジェクションが通った原因は“静的解析ツールのバージョンアップ”　「自動修正機能」に気をつけるべき理由

登壇者の自己紹介

小川氏：小川と申します。「静的解析ツールで生まれたSQLインジェクション」というタイトルで発表いたします。よろしくお願いします。

自己紹介を簡単に。経歴ですが、昔学生の時にWebアプリ開発のバイトをしていて、就職後は10年ぐらいぜんぜん違う、製造業で働いていました。ずっとパソコンを見ていたら目が悪くなるかなと思ってほかの業界に行ったのですが、結局ずっとExcelやWordを見ていて、あまり変わりませんでした。

結局やはりITだなと思って、最近root ipという会社に転職して、BtoBのSaaSを作っています。PHPとVueがわかる人が今本当に欲しいので、誰か心当たりがあったら来てください。

静的解析ツールでSQLインジェクションが生まれてしまった事例

おもしろかった脆弱性というところで、今日はCVE-2023-22727というものを紹介します。

これは何かというと、PHPのフレームワークのCakePHP 4で、SQLインジェクションがあったというものです。ORMのlimit、offsetというところでSQLi（SQLインジェクション）が通った。CVSSとしては9.8という、最近「curl」の作者がブチ切れていたやつと同じぐらいの危険度です。

（会場笑）

すでに半年以上前から修正しているので、もう出してもいいかなというやつです。

PHPをあまり使っていないとご存じないかもしれませんが、CakePHPは、たぶんLaravelの次ぐらいに使用率が多いフレームワークで、けっこう日本でも使われているので、わりかし危なかったんじゃないかなというやつです。わりかし使いやすいやつですね。

コード品質が上がる静的解析ツールを使ってフレームワークのコードをきれいにした時に逆に（インジェクションが）発生したという、ちょっとおもしろい事例というところで、今日持ってきました。

脆弱性の説明

みなさんは脆弱性に興味があると思うので、まず脆弱性から説明します。

（スライドを示して）こんなもんで、ごく普通のORMです。簡単にORMの説明をしておくと、下のようなSQLをプログラミング言語っぽくこんな感じで書いたら、よしなに作ってくれるやつですね。

ここのlimitに、こんな感じのTRUNCATEとかを適当に放り込んだら、ごく当たり前にSQLiが通ってしまったというやつです。

（会場笑）

ユーザー入力を検索結果が入るところに放り込んだら死んだという事例です。

「マジで？」と思ったんですが、バージョン4.2というけっこうなバージョンで、今さらなんでこんなのが出てくるんだろう、とか。

（会場笑）

そもそもこれは、脆弱性じゃなくて仕様じゃないのかなとか。

（会場笑）

あと、これを見たら、たぶんユーザー入力をそのまま使うな、とか、そういう思いをいろいろ持つと思うんです。

バージョンアップをしたらSQLiが通るようになった

これにはちょっと経緯があって。もともとバージョン4.2まではintにキャストされていたんですね。だから何を放り込んでも、死にはしなかった。

その仕様を知っていればサニタイズしなかったので、バージョンアップすると逆にヤバかったという罠があったということです。

何が起きていたのかと思って、当時のコミットログを見てみました。簡単に解説すると、limitというORMの関数にnumという引数を入れて、こいつがnullかobjectじゃなかったらintにキャストするというやつがあったんですが、こいつが消えていました。

「何や、これは？」というところで、ちょっと現場猫感を感じながら。

（会場笑）

エラーを指摘・修正してくれる「静的解析ツール」

その原因を追究したのがここからです。

ちょっと話は変わるのですが、静的解析ツールをみなさんご存じでしょうか？　開発をやっている方は、きっとご存じだと思います。コミットメッセージで「Fix errors reported by psalm」という、「Psalm」というものがあって、これはPHPの静的解析ツールです。なので、これは静的解析ツールのエラー修正の時に発生したというものになっています。

これが何をやってくれるかというと、型がおかしいよとか、ロジックとか、この変数使っていないよとか、動かす前にまず静的にこれをいろいろ調べて、しかも修正もしてくれるという、だいぶすごいツールです。最近の開発はみんなこれを使っていて、「これがあったらどんどんバグが減るぞ」みたいなすごいツールです。

intキャストが消えた理由

これがめちゃめちゃ指摘をしてくるんですね。これが起きた時のフレームワークで試しにやってみたら342件エラー、と言われて。これはきついなと。

自動修正機能があるので、たぶん「手でやっていられない」と思って、バッとやったと思うんですよ。僕も試しにやってみたところ、intキャストが消えました。

（会場笑）

これかと。たぶん、intキャストが消えて、ただの代入になったので、さらにこのif文も消えてコミットされたのかなと。

「これは何や？」と思いました。ちょっといろいろあるのですが、PHPにも引数の型宣言機能があります。

（スライドを示して）例えばこんな感じで、「int $num」とか。ただちょっと、objectも受けられるかたちで、古いPHPはunion型が非対応だったので、たぶんあえてint | objectみたいなものを書かずに、Docコメントという、IDEとか静的解析でチェックしてくれるコメントで教えるだけに留めるとやったっぽいんですね。

静的解析ツールは、こいつを信じて「キャスト要らないな」と消してしまったらしく、でも、Docコメントは強制じゃないので、string型でも通って、無事死亡したというアレですね。

この自動修正はヤバいんじゃないのと、さすがにissueを上げようかと思って調べたのですが、このRedundantCastなんちゃらかんちゃらというやつを調べたら、同じところで出ているやつの9割方は大丈夫だったんですね。

外部とのインターフェイスで、強制がないところがちょっヤバかったという感じで、union型が使えないPHPはもう死んでいくので、まぁ、いいかなということでちょっと流しました。

まとめ

ということでまとめですが、コード品質が上がる静的解析ツールでたまに脆弱性が生まれることがあります。

開発の人にとっては、やはりこいつは正義なので使いましょうということなんですが、自動修正、特にインターフェイス部分ではちょっと気をつけましょう。

あと、ここに書き忘れたのですが、MySQLとかだったら複文実行を禁止している、クエリを2個以上投げられない機能があるので、それを有効にすれば、こういう、後ろにTRUNCATEみたいな、めちゃくちゃやってくるやつは防げます。テストのために無効にして、これを投げたら本当に通ったのでめっちゃビビったんですけど。

（会場笑）

僕、通らないって知っていたんですよ。もともとキャストされると知っていたから。まさにこの、知っていた人間がかまされたというやつで、めっちゃビビりましたが、気をつけましょう。ユーザー入力もきちんとチェックしましょうという話ですね。

あと、ちょっとオブラートに包んで、セキュリティ研究者向けの話で、「GitHub」で「静的解析かけた」とか、ツール名が入ったコミットを漁ったら、きっとおもしろいものが出てくるんじゃないかなと思っています。

（会場笑）

あと、検索してもぜんぜん見つけられなかったんですが、（スライドを示して）ほかのOSSではこんな感じで、同じように「静的解析ツールに言われたから直したわ」みたいなコミットメッセージで爆発したやつがあったような気がしたのですが、誰か知っていたら教えてください。ちょっと思い出せませんでした。というわけで終わりです。

資料は「Twitter（現「X」）」の「@hoge」というところに置いておきますので、よろしくお願いします。以上です。

（会場拍手）