LoginSignup
8
6

More than 5 years have passed since last update.

LaravelCollectiveのFormファサードを使用している場合にinputの値をGETパラメータから変更出来てしまう

Last updated at Posted at 2018-07-04

タイトルの通り、LaravelCollectiveのFormファサードを使用してinputフィールドを作成している場合、nameと同じ値のGETパラメータをつけるとvalueの値を直接変更出来てしまうようなので調べてみた。

追記 (2018/07/06)

コメントで指摘いただきましたが、5.4、5.6(5.6.9以上)ではこの挙動はデフォルトではオフになっており、オプション扱いになったようですf。

以下のように considerRequesttrue とすると、GETパラメータから値を変更出来るようになります。
(デフォルトでは falseです)

{{ Form::considerRequest(true) }}

このプルリクが5.4に対して出されており、マージ後、(その時点での)最新のもの(5.6.9)にマージされたため、5.4と5.6.9以降、にはこのオプションが適用されています。(5.5にはないので注意)
(ドキュメントにはこのオプションのことは書かれていないので注意が必要…。)

ですので、以下の説明は適用されていないバージョンに対しての話となりますのでご注意下さい。
最新のバージョンだと問題ありません。

再現

LaravelCollectiveを入れて(5.2以上)、以下のようにBladeに記述する。

{{ Form::text('test') }}

ページにアクセスすると以下のようなHTMLが生成される。

<input name="test" type="text">

で、このページに以下のようにクエリをつけてアクセスする。

https://example.com/?test=hoge

するとvaluehogeが入った状態でHTMLが生成される。

<input name="test" type="text" value="hoge">

原因

LaravelCollectiveの仕様。
ドキュメントにも書いてある。

So, the priority looks like this:

  1. Session Flash Data (Old Input)
  2. Data From Current Request (via Request::input method)
  3. Explicitly Passed Value
  4. Model Attribute Data

以下のコードが該当箇所。

$request = $this->request($name);
if (! is_null($request) && $name != '_method') {
    return $request;
}

5.2から導入されたようで、以下のプルリクで実装されたよう。
Fill values by current Request by decadence · Pull Request #217 · LaravelCollective/html

懸念点

懸念点としては以下などがあると思う。

以下のstackoverflowでも書いている人がいた。
laravel - Stop LaravelCollective Form Model Binding being overwritten by GET Variables? - Stack Overflow

hidden値の書き換え

Form::inputのファサード内に適用されているため、hiddenフィールドにも上記仕様は適用される。
よって、getパラメータからhiddenのvalueを任意の値に設定出来てしまう。
すぐさまセキュリティのリスクになるわけではないけれど意図しない動作をしてしまう可能性がある。

htmlspecialcharsによるexception

valueの値はHTMLを生成する際にhtmlspecialcharsにかけられるので、XSSが発生することはないのだけど、そのせいで配列を渡すとexceptionを吐いて落ちてしまう。
以下、value値をeで生成している箇所
https://github.com/LaravelCollective/html/blob/5.6/src/HtmlBuilder.php#L463

前述の例だと、以下のようなURLでアクセスするとページがexceptionを吐く。

https://example.com/?test[]=hoge

htmlspecialchars() expects parameter 1 to be string, array given

代替案

フォーム部分のためにLaravelCollectiveを使用しているのなら、使用しない、という選択もありだと思う。
例えば、Form::open()のファサードはCSRF用のトークンも自動で生成してくれるので便利だが、Laravel5.6からは、Bladeに@csrfでトークンを生成してくれるディレクティブが追加されたりしている。
(5.3以降ならcsrf_fieldヘルパ関数でも生成出来る)

エラー時や確認ページから戻った際の前回の入力値 (Session Flash Data) は、old()で入れておけばよい。

<input type="text" value="{{ old('test') }}">

oldの第二引数にデフォルトの値も設定可能なので、既存のレコードの編集フォームも対応出来る。

ファサードの上書き

あまりよくはないと思うけど、ファサードの上書きで、該当のrequestからの取得箇所を消してしまう事も出来た。

参考 : Best way to overwrite option method from Laravel 5 FormBuilder class?

どこでもよいが、例えば、app/Service/Html/以下に作成する。
上書き用に元のHtmlServiceProviderをextendsしたプロバイダクラスを作成し、カスタムのFormBuilderを読み込ませるようにする。

app/Service/Html/HtmlServiceProvider.php
<?php

namespace App\Service\Html;

class HtmlServiceProvider extends \Collective\Html\HtmlServiceProvider
{
    protected function registerFormBuilder()
    {
        $this->app->singleton('form', function ($app) {
            $form = new FormBuilder($app['html'], $app['url'], $app['view'], $app['session.store']->token(), $app['request']);

            return $form->setSessionStore($app['session.store']);
        });
    }
}
app/Service/Html/FormBuilder.php
<?php

namespace App\Service\Html;

use Illuminate\Foundation\Http\Middleware\ConvertEmptyStringsToNull;

class FormBuilder extends \Collective\Html\FormBuilder
{
    public function getValueAttribute($name, $value = null)
    {

        if (is_null($name)) {
            return $value;
        }

        $old = $this->old($name);

        if (! is_null($old) && $name !== '_method') {
            return $old;
        }

        if (function_exists('app')) {
            $hasNullMiddleware = app("Illuminate\Contracts\Http\Kernel")
                ->hasMiddleware(ConvertEmptyStringsToNull::class);

            if ($hasNullMiddleware
                && is_null($old)
                && is_null($value)
                && ! is_null($this->view->shared('errors'))
                && count($this->view->shared('errors')) > 0
            ) {
                return null;
            }
        }

        // ここに request から取得するコードがあったので削除

        if (! is_null($value)) {
            return $value;
        }

        if (isset($this->model)) {
            return $this->getModelValueAttribute($name);
        }
    }
}

あとは、app.phpHtmlServiceProviderを読み込んでいる箇所を変更。

config/app.php
'providers' => [
    ...
    //Collective\Html\HtmlServiceProvider::class,
    App\Service\Html\HtmlServiceProvider::class,

これであとは普通に使用すればrequestからは取得されなくなる。

感想

Laravel4.x系まではLaravelの中に組み込まれていたけれど、5.x系から分離されたコミュニティベースでの更新に移行されたよう。
便利ではあるけれど、使わないと実装出来ない、というほどのものでなければ使わないほうがむしろシンプルになってよいのかもしれない。

ちなみに、2018/7/4現在、LaravelCollectiveの公式サイトが証明書の期限が切れてしまっているよう。
https://laravelcollective.com/
GitHubにissueも上がってた。
Website security cert expired. · Issue #546 · LaravelCollective/html

8
6
2

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
8
6