[CakePHP3] バリデーションの落とし穴

早いもので 2018 年ももう師走ですね。
さて、こんな年末の忙しい時に開発案件が舞い込んで、とある予定管理システムを受注したとしましょう。
年末の忙しい時ですから、当然、フレームワークは RAD に CakePHP3 を使いたいはずです。そうですね？

日付時刻を比較するバリデーション

まずは予定の登録画面を手早く作ってしまいましょう。
開始日時と終了日時が選択できて予定内容が登録できればひとまず今日の仕事は終わりでいいでしょう。 bake コマンドを使えば簡単ですね。

あまりにも簡単だったので、開始日時が終了日時よりも前にならないようにバリデーションくらいは書きましょうか。

$validator->add('end_datetime', 'date_range', [
    'rule' => function($value, $context) {
        return $value > $context['data']['start_datetime'];
    },
    'message' => '開始日時以降を選択してください',
]);

終了日時を開始日時よりも前にすると、無事にエラーメッセージが表示されました。簡単ですね。

いいえ。事はそれほど単純ではないのです。
上のコードは間違っています。なぜなら、バリデーションルールが受け取るのはこんなデータだからです。

[
    'year' => '2018',
    'month' => '12',
    'day' => '01',
    'hour' => '00',
    'minute' => '00',
    'second' => '00',
];

どうやら、思いがけず配列に対して比較演算子を使っていたようですね。

PHP において配列同士を比較する場合、要素同士を個別に比較するルールが一応あります。問題のコードが一見、正しいふるまいを見せたのはそのためです。
しかし、要素の数を比較するルールの方がそれに優先されます。したがって、問題のコードは次のような入力に対して期待通りに動作しません。

// start_datetime
[
    'year' => '2018',
    'month' => '12',
    'day' => '01',
    'hour' => '00',
    'minute' => '00',
    // 'second' がないので要素数が少ない
];

// end_datetime
[
    'year' => '2017',
    'month' => '12',
    'day' => '01',
    'hour' => '00',
    'minute' => '00',
    'second' => '00',
];

仕方ないので配列を DateTime に変換する便利関数でも作りましょう。

function arrayToDateTime(array $a) {
    $a += [
        'year' => 0,
        'month' => 1,
        'day' => 1,
        'hour' => 0,
        'minute' => 0,
        'second' => 0,
    ];
    $d = new \DateTime();
    $d->setDate($a['year'], $a['month'], $a['day']);
    $d->setTime($a['hour'], $a['minute'], $a['second']);
    return $d;
}

できました。
DateTime クラスは比較演算子をオーバーロードしていますので、バリデーションルールの中でこの関数を使うことで、きっと簡単に日付時刻の比較が行えるようになるに違いありません。

バリデーションの落とし穴

ところで、次のような入力を受けた場合はどうなるでしょう？

// start_datetime
[
    'year' => '2018',
    'month' => '11',
    'day' => '30',
    'hour' => '23',
    'minute' => '00',
    'second' => '00',
    'timezone' => '+00:00',
];

// end_datetime
[
    'year' => '2018',
    'month' => '12',
    'day' => '01',
    'hour' => '00',
    'minute' => '00',
    'second' => '00',
    'timezone' => '+09:00',
];

タイムゾーンの違いを考慮すると、終了時刻が開始時刻よりも前になっているように見えます。
こうした入力を受けた場合に、データベースにどんな値が保存されるかご存知ですか？

多分、ほとんどの方はご存知ではないだろうと思います。

――って、そんなことでいいのでしょうか？
データベースにどんな値が保存されようとしているのか、バリデーションの段階になっても分からない――？
それで、どうやって不正な値ではないかどうかを検証しろというのでしょう？

CakePHP のバリデーションって、昔からこんなで仕様でしたっけ？

いいえ、違います。以前はこんな仕様ではありませんでした。

CakePHP2 までは日付時刻の配列はバリデーションの前にすでに文字列に変換されていました。
ところが CakePHP3 になって、この種のデータ変換がバリデーションの後に行われるように仕様変更されたのです。
そして、この仕様変更はさらなる問題を引き起こすものでした。

予定管理システムに話を戻します。
CakePHP に仕様を変更された意趣返しに――というわけではありませんが、予定管理システムの方も仕様を変更して終了日時については省略できるようにしてみましょう。
なぜそんな仕様にするのかというと、そうしないと話が進まないからです。

$validator->allowEmpty('end_datetime');

理由はともかくとして、これで終了日時を省略できるようになりました。
実際に未選択のままにしてフォームを送信すればちゃんとそのように動作するはずです。

でも、考えてみると不思議です。なぜこれで動作するんでしょう？
これまで見てきたように CakePHP3 ではバリデーションの段階では日付時刻は配列のままだったはずです。
すると、バリデーターは次のような入力値を空と判断したことになります。

[
    'year' => '',
    'month' => '',
    'day' => '',
    'hour' => '',
    'minute' => '',
    'second' => '',
];

日付時刻型のフィールドであれば、これを空と認めてもいいでしょう。
でも JSON 型のフィールドであれば、これは空とは認めがたい内容です。
いったい CakePHP3 はどのような仕組みで入力値が空かどうか適切に判断しているのでしょう？

実は、誠に遺憾ながら CakePHP3 は適切にではなく適当に判断します。
入力が配列であり、かつ year または hour というキーがあれば日付時刻配列と見做し、この場合に配列の要素をすべて結合して空文字列になれば、バリデーターは入力を空だと判断してしまいます。
この時、日付時刻型のフィールドかどうかは問われません。なぜなら、バリデーターはデータベース上のデータ型なんて知らないからです。

入力を空と判断した場合、バリデーターは次のようにふるまいます。

• 空を許可しないフィールドの場合、登録されたバリデーションルールを呼ばずに NG とする。
• 空を許可するフィールドの場合、登録されたバリデーションルールを呼ばずに OK とする。

前者のふるまいも問題がないわけではありませんが、より大きな問題となるのは後者のふるまいです。

たとえば、空を許可するフィールドに次の入力を送り付けてみましょう。

[
    'year' => '',
];

配列で year というキーがあり、要素は空文字列ですので、この入力は空です。したがって、バリデーションルールはお呼びではありません。
空を許可するフィールドに対してこの入力は正当です。バリデーションを通過します。

しかし、この時、データベースに保存される値も空になるとは限りません。
実際、整数型の場合には 1 が保存されます。
これは NULL を許可する外部キー（たとえば parent_id のようなフィールド）に問答無用で 1 を投入できることを意味します。
アプリケーションの仕様によっては、これは致命的な欠陥になりかねません。

影響を受けるバージョン

CakePHP 3.0.0 から 3.6.13 までのすべてのバージョン

対応

SecurityComponent を有効にするのが一案です。任意の構造の配列を送ることができなくなりますので、おそらく防御策になるだろうと思います。

また CakePHP 3.7 になるとこの問題はある程度解消されており、こうした場合、ほとんどのデータ型では NULL が保存されるようになります。
厳密には 3.7 でも依然、バリデーションを迂回可能なのですが、影響を受けるデータ型は少なくなったと思います。
今年中にはリリースがあると思いますので、リリースされたらアップグレードを行うとよいかもしれません。

また 3.7 ではどんな値を空とするかを厳密に判定するために allowEmptyString() あるいは allowEmptyDateTime といったメソッドが追加されています。
これらのメソッドは一応は私が追加したものではあるものの、途中であまりよい解決方法ではない気がしてきてプルリクエストを取り下げたのですが、最終的に取り込まれたようですね。
使えば安全になるとは思うので、使ってみてもよいと思います。

なお、この問題は紆余曲折があり、脆弱性ではないということになりましたので、過去のバージョンについては修正は行われません。
何らかの事情でアップグレードできない方は Model.beforeMarshal イベントを捕まえて、データをフィルタリングするとよいかもしれません。
Table クラスはフィールドのデータ型を知っているはずですから、わりと汎用的なフィルターを作ることができるのではないかと思います。

終わりに

CakePHP Advent Calendar の初日ということで、明るい話題の方が良かったかなとも思ったのですが、一方で、これはおそらくほとんどすべてのアプリケーションが影響を受ける問題ですので、早めにお伝えした方が良いかなとも思い、初日を選んでみました。
この記事が少しでも皆さまのお役に立てば幸いです。