Laravel 8.47 で修正された RequiredIf のセキュリティ上の問題箇所をチラ見してみる

まえがき

Laravel 8.47で、2つのセキュリティ上の問題点が修正されました。
どちらもバリデーション絡みで、1つは、'active_url' もう1つが、RequiredIf。

'active_url'の方はややマイナーなのでおいておき、
RequiredIf の問題だった箇所をチラ見してみたいと思います。

参考：[6.x] Type hinted arguments for Illuminate\Validation\Rules\RequiredIf #37688

参考：GitHub Fixed dns_get_record loose check of A records for active_url rule #37675

本題

一応補足しておきますと、文字列の 'required_if' というのがありますが、それをより柔軟に使えるようにする為に、RequiredIf ルールクラスがあります。RequiredIf を使っても、最終的には、'required' か空文字列に置き換えられます。

修正前と修正後のファイルの比較です。

修正前

class RequiredIf
{
    /**
     * The condition that validates the attribute.
     *
     * @var callable|bool
     */
    public $condition;

    /**
     * Create a new required validation rule based on a condition.
     *
     * @param  callable|bool  $condition
     * @return void
     */
    public function __construct($condition)
    {
        $this->condition = $condition;
    }

    /**
     * Convert the rule to a validation string.
     *
     * @return string
     */
    public function __toString()
    {
        if (is_callable($this->condition)) {
            return call_user_func($this->condition) ? 'required' : '';
        }

        return $this->condition ? 'required' : '';
    }
}

修正後（コンストラクタのみ）Ver.8.47.0の場合

    public function __construct($condition)
    {
        if (! is_string($condition) && (is_bool($condition) || is_callable($condition))) {
            $this->condition = $condition;
        } else {
            throw new InvalidArgumentException('The provided condition must be a callable or boolean.');
        }
    }

修正前は、$condition の値をチェックしていない為、ユーザーから任意の関数名などが渡された場合、それらが実行されてしまいますね。

（この変更が breaking change と言っている人もいる為、今後更に改良されるかも知れません）

そう言えば、以前も…

'unique'なんかでも、ユーザーからの値を直接渡すなという話がありましたね。
参考：日本語ドキュメント（unique）

Note: ユーザーがコントロールするリクエストの入力をignoreメソッドへ、決して渡してはいけません。・・・（以下略）・・・

ということで、バリデーションする際にノーチェックでバリデーションメソッドに値を渡すのは辞めましょう。（というか、ユーザーからの入力値は基本渡さないように、という事だと思いますが）

ところで

懸賞金じゃないですが、オープンソースの脆弱性を報告して、報酬がもらえるというサイト（仕組み）があるようですね。
https://www.huntr.dev/

今回もそちら経由で、報告している人には報酬が支払われるようです。

話はやや脱線して

RequiredIf って、たまに使ったりしますが、
'required' じゃない時は、'nullable' を出力してくれると嬉しかったりしますね。
追加でバリデーションする場合、結局必要になったりもしますから。

（現状の例）

'email' => [new RequiredIf(xxx), 'nullable', 'email:filter'],

まあ、文字列の 'required_if' との絡みもあるから、仕方無いと思いますが。

雑感

やはり、入力チェック大事ですね。

おかしな箇所ありましたらコメント下さい。