Androidが再生産しているSQLインジェクション？

※ちょっと釣りタイトル気味ですが真面目な話です。
これからするのは、ちょっとセキュリティ屋さんやスマホアプリ開発現場の人達の頭が痛くなるかもしれないお話です。*1

定番のSQLインジェクション対策といえば「prepared-queryを利用し、パラメータはbindメカニズムを使って与える」ですよね。*2

Androidでアプリを開発する場合、その方法は使えるのでしょうか？
答えはYesです。AndroidはデータベースにSQLiteを使用しており、なおかつprepared-queryを利用出来ます。例えばこんな感じで。

SQLiteDatabase db = databaseHelper.getWritableDatabase(); //←databaseHelperはSQLiteOpenHelperを継承したクラスのオブジェクト

SQLiteStatement stmt = db.compileStatement("INSERT INTO cat(name, age, weight) VALUES(?, ?, ?)");
stmt.bindString(1, "tama");
stmt.bindLong(2, 1);
stmt.bindLong(3, Long.parseLong("4"));
stmt.executeInsert();

上記を見て「どこにも問題無いじゃん。仮にbind知らない奴がSQLインジェクションやらかしたとしても、Androidの責任じゃないだろ。タイトル詐欺じゃね？」って思った方、早まらないで続きをご覧ください。

android.database.sqlite.SQLiteDatabaseというクラスには、以下のようなメソッドが用意されています。

public Cursor query(String table, String[] columns, String selection, String[] selectionArgs, String groupBy, String having, String orderBy)
public int delete(String table, String whereClause, String[] whereArgs)
public int update(String table, ContentValues values, String whereClause, String[] whereArgs)

どうやら単純なテーブル読み書き時にSQL全部を記述しなくて済ませる為のメソッドのようですね。
これらのメソッドのwhereClauseとwhereArgsには

String whereClause = "name = ?";
String[] whereArgs = new String[] {"mikenyan"};
db.delete("cat", whereClause, whereArgs);

といった値をセットして使い、内部で「DELETE FROM cat WHERE name = 'mikenyan'」というSQLを実行する仕組みになっています。
selectionとselectionArgsも名前が違うだけで同じ役割の引数になっていますね。
また同様の役割を果たす引数が、android.content.ContentProviderにてOverride必須なquery・update・deleteでも用意されています。関連するContentResolverでも同様です。*3

ところが、ここで少し残念な仕様が判明します。whereArgsはStringの配列として扱われていて、内部で最終的にbindStringが呼び出される仕様になっています。
つまるところ、longやdouble等の値を直接bindする事が出来ない仕様になっています。
もし全てのパラメーターでbindメカニズムを利用したい場合には

String whereClause = "name = ? AND age = ?";
String[] whereArgs = new String[] {"mikenyan", Long.toString(1)};
db.delete("cat", whereClause, whereArgs);

のようにして「DELETE FROM cat WHERE name = 'mikenyan' AND age = '1'」と自動的な暗黙の型変換の動作にドキドキしながら実行するしか出来ないようです。

では厳密に「DELETE FROM cat WHERE name = 'mikenyan' AND age = 1」というSQLを(query/delete/updateメソッドで)実行させるにはどうすればいいのでしょうか？　答えは、次のコードになります。

String whereClause = "name = ? AND age = " + Long.toString(1);
String[] whereArgs = new String[] {"mikenyan"};
db.delete("cat", whereClause, whereArgs);

……値の直接埋め込み、いわゆる文字連結によるSQL組み立てになってしまいましたね。とはいえ、文字列だけをきっちりbindし、SQLインジェクションを回避するのは十分可能です。あくまで『分かってる人が使う限り』という条件付きですけども…。

ここからがタイトルが懸念している本題になります。まず、プログラム初心者が学習する流れを考えてみてください。

先程のサンプルコードは

String whereClause = "name = '" + "mikenyan" + "' AND age = " + Long.toString(1);
String[] whereArgs = new String[] {};
db.delete("cat", whereClause, whereArgs);

と書き、whereArgsを一切使わないでも同じ事が出来ます。
しかも要素が0個のString配列を指定せずnullを指定する事が可能な為、次のようなものも「SQLインジェクションの問題が無いコード」として書かれたりします。

db.delete("dog", "id = " + Long.toString(id) , null);

また「SQLインジェクションの起こりえない文字列値を渡す仕様」の場合に

db.delete("mouse", "type = '" + TYPE_DREAM + "'" , null); //TYPE_DREAMは"dream"という文字列の定数

と文字連結で指定するコードも書く場合があるようです。これもそのアプリ単体では「SQLインジェクションの問題が無いコード」ではあります。

しかしプログラム初心者がそれらのコードを見た結果「whereArgsにbindしたい文字列の配列を入れる」という仕組みそのものに気づかないまま「whereClauseに検索条件の文字列を入れて、whereArgsにはnull指定しとけば良いんだな！」のように微妙に勘違いした状態で理解してしまいかねないのです。
そうしてその初心者は「正しくコピペ」しているつもりでSQLインジェクション入りのコードを書いてしまう可能性が出てくる訳です。

//初心者が間違ってその後書きそうなコード
EditText editTextArtist = (EditText)findViewById(R.id.EditTextArtist);
String artist = editTextArtist.getText().toString(); //B'zやI'veやL'arc-en-Cielとか入る
db.delete("song", "artist = '" + artist + "'" , null);

まあそれでも運良くデバッグ時に「あ、これだとSQLインジェクションになるんだ」と学習する機会があるならば、そのコードを書いた初心者はSQLインジェクションを作り込まないよう成長してくれるでしょう。ですが世の中、必ずしもそういうケースばかりにはなりません……。
コーディング担当とデバッグ担当が別人でコーディング担当に情報がフィードバックされなかったり、テスト仕様書がSQLインジェクションを想定して無かったり、デバッグ担当にSQLインジェクションの知識が無かったり、そもそもプロジェクトメンバー全員がSQLインジェクションの事を知らなかったり、そういった不幸な現場も残念ながら存在しています。もしそのまま他の開発現場にSQLインジェクションを知らないまま「プログラム経験者」として流れていったら……そこいらでSQLインジェクションなコードが量産されてしまうでしょう。

こういう懸念を感じた為、このエントリのタイトルを「Androidが再生産しているSQLインジェクション？」とさせて頂きました。m(_ _)m

誤解の無いように言っておきますが、AndroidのSQLiteDatabaseクラスやContentProvider＆ContentResolverクラスは「SQLインジェクション対策が出来てない」訳ではありません。
あくまでメソッドの仕様が「結果的にコピペしたがるプログラム初心者がSQLインジェクションを作り込んでしまいやすい」だけなんです。
個人的には、最初から型指定が可能なメソッドを一緒に用意し、型指定を省略した場合のみ「全て文字列としてbindする」仕様の方が良かったなーと思います。

public int delete(String table, String whereClause, String[] whereArgs, String[] types) //←こんなのを一緒に用意
public int delete(String table, String whereClause, String[] whereArgs)

おそらくは「正しく書く方法があるんだから、書く側が注意するだけの問題だろ」という風に「分かってる人」側は思うでしょう。ですから、今後のAndroid APIがバージョンアップしても「Stringだけbindしろ」という仕様は継続されると思われます……。*4

API仕様がそうならそれに従わざるを得ないので、次善策として

• bindする値が無い時でもwhereArgs／selectionArgsにはnullを指定せず、必ずString配列を渡す
• コメントに注意点を書いておく

というルールでコードを書く方がベターでは無いかと思われます。（見る人の事を考える余裕があるなら）

String whereClause = "name = ? AND age = " + Long.toString(1); //文字列以外は直接埋め込み、文字列は?でプレースホルダ指定
String[] whereArgs = new String[] {"mikenyan"}; //文字列はbindを使って指定
db.delete("cat", whereClause, whereArgs);

String whereClause = "id = " + Long.toString(id); //文字列以外は直接埋め込み、文字列は?でプレースホルダ指定
String[] whereArgs = new String[] {}; //文字列はbindを使って指定
db.delete("dog", whereClause, whereArgs);

まあ「再生産する」と大げさに言っても絶対数が（SQLインジェクションが大量生産されてた時代よりは）少ないでしょうけどねー。*5

*1:仕事の種になると逆に喜ぶ方もいるかもしれませんが。

*2:どうしても駄目な環境だと次善策としてescape処理しますが

*3:ただし渡されたselectionとselectionArgsをどう扱うかはContentProvider実装者の自由で、使わずに無視する事も可能ではあります。

*4:あと、ContentProvider＋ContentResolverの仕様を今から変えるのは非現実的でしょうしね…

*5:理想論を言うならば、技術者たるもの「自分でSQLインジェクションの可能性に気づく」くらいして欲しいところですが…。ところで「C経験者で\とつきあった事ある人が、SQLインジェクションやXSSを全く気にしてない」って何故起こるんでしょうね？