Born Too Late

Yuya's old tech blog.

CakePHP の alphaNumeric バリデータにパッチを送った話

2013-03-17 14:20:46

ちょっと CakePHP を使う機会があったのでソースコードを眺めていたところ、ちょいとしたバグが見つかりまして、すぐにパッチを書き Pull Request を行ったところ、ものの数時間でマージされました。

バグというのが、「alphaNumeric バリデータにおいて、改行で区切ってしまえばどんな文字列も通してしまっていた」ということです。

修正は以下の 1 コミットのみ。

Fix alphaNumeric validation · 14c81fe · cakephp/cakephp

preg_match や preg_replace をはじめ、PCRE を使った正規表現で、このような 1 行の文字列にバリデーションを行う際は、正規表現の修飾子として D (PCRE_DOLLAR_ENDONLY) を使用しないと、末尾の改行に対して検出漏れが発生してしまいます。

それどころか、m (PCRE_MULTILINE) なんてつけていると各行に対してパターンマッチングを行い、preg_match の場合はどれかひとつでもマッチすれば 1 を返すので、例えば以下のような文字列も OK、という恐ろしいことが起こってしまいます。

$result = preg_match('/^[\p{Ll}\p{Lm}\p{Lo}\p{Lt}\p{Lu}\p{Nd}]+$/mu', "abc\n<script>alert(1)</script>");
var_dump($result);
// => int(1)

あくまでユニットテストレベルでの検証ですが、このような恐ろしいことが起こっておりましたので、早いところ次のリリースが行われることを願うばかりです。
CakePHP 2.3.2 当たりで取り込まれることが予想されます。

こういう正規表現を書いていると徳丸先生方面から斧が飛んで来る可能性があります。
予め徳丸本等を読んで対策を行うことをオススメ致します。
あとは PHP のマニュアルとか PCRE 自体のマニュアルの PASSING MODIFIERS TO THE REGULAR EXPRESSION ENGINE 付近とか。
(なお、徳丸本の 81 ページによると ^ の代わりに ¥A を、$ の代わりに ¥z を使って、「行の」先頭・末尾ではなく「データの」先頭・末尾を示すようにせよ、とあります。)

なお、その他のバリデータも同様の問題をはらんでいる可能性もありそうですが、普段 CakePHP を使わない私としてはそこまで見切れないので、仕事等で CakePHP を利用している方は、是非とも contribute してみてはいかがでしょうか。