本当は危ないCodeIgniter4のModel::update()

この記事は CodeIgniter Advent Calendar 2022 - Qiita の24日目です。まだ、空きがありますので、興味のある方は気軽に参加してください。

CodeIgniter Model

CodeIgniter4には CodeIgniter Model が追加されました。

これは、データベース内の 1つのテーブルをより便利に扱うために、よく使う便利な機能や追加機能を提供するものです。

Model::update() の仕様

Model::update($id, $data) はデータベースのレコードを更新するためのメソッドです。

このメソッドには、多くの開発者が予期しない仕様があり、想定していない値を渡された場合、データベース内のデータを破壊される可能性があります。

それは、$id に null などを渡すと（他にWHERE句を指定するメソッドを呼んでいない場合に）、全レコードが更新されるというものです。

つまり、以下はWHERE句のないUPDATE文を実行します。

$this->model->update(null, $data);

なお、CodeIgniter 4.3.0 からは、WHERE句のないUPDATEの場合はエラーになるように変更される予定です。

脆弱性を作らないために

基本的なセキュリティ対策ですが、 IDに想定していない値を渡さないように、確実にバリデーションすることです。

脆弱性のあるコードの例

せっかくなので、脆弱性のサンプルを作成しました。以下のようなコードは書いてはいけません。

実際に動作するコードをGitHubに置いてあります。

https://github.com/kenjis/ci4-model-update-danger

興味のある方は、攻撃方法を考えてみてください。

脆弱性のあるコード例(1)

POSTデータからIDとデータを受け取るパターンですが、IDのバリデーションがされていません。

<?php

namespace App\Controllers;

use App\Models\NewsModel;
use CodeIgniter\Exceptions\PageNotFoundException;
use CodeIgniter\HTTP\ResponseInterface;

class News extends BaseController
{
    private NewsModel $model;

    /**
     * Validation Rules
     */
    private array $rules = [
        'title' => 'required|min_length[3]|max_length[255]',
        'body'  => 'required|min_length[10]|max_length[5000]',
    ];

    public function __construct()
    {
        $this->model = model(NewsModel::class);
    }

    // ...

    /**
     * @return ResponseInterface|string
     */
    public function update()
    {
        $id = $this->request->getPost('id');

        if ($this->request->getMethod() === 'post' && $this->validate($this->rules)) {
            $title = $this->request->getPost('title');
            $slug  = url_title($title, '-', true);

            $data = [
                'title' => $title,
                'slug'  => $slug,
                'body'  => $this->request->getPost('body'),
            ];
            $this->model->update($id, $data);

            return $this->response->redirect(site_url('news/' . $slug));
        }

        return $this->edit($id);
    }
}

脆弱性のあるコード例(2)

自動ルーティング（改善）を使ったケースで、IDは引数で受けています。これも、IDのバリデーションがされていません。

<?php

namespace App\Controllers;

use App\Models\NewsModel;
use CodeIgniter\Exceptions\PageNotFoundException;
use CodeIgniter\HTTP\ResponseInterface;

class News2 extends BaseController
{
    private NewsModel $model;

    /**
     * Validation Rules
     */
    private array $rules = [
        'title' => 'required|min_length[3]|max_length[255]',
        'body'  => 'required|min_length[10]|max_length[5000]',
    ];

    public function __construct()
    {
        $this->model = model(NewsModel::class);
    }

    // ...

    /**
     * @param false|int $id
     *
     * @return ResponseInterface|string
     */
    public function postUpdate($id = false)
    {
        if ($this->validate($this->rules)) {
            $title = $this->request->getVar('title');
            $slug  = url_title($title, '-', true);

            $data = [
                'title' => $title,
                'slug'  => $slug,
                'body'  => $this->request->getVar('body'),
            ];
            $this->model->update($id, $data);

            return $this->response->redirect(site_url('news2/view/' . $slug));
        }

        return $this->getEdit($id);
    }
}

脆弱性のあるコード例(3)

POSTデータからIDとデータを受け取るパターンで、IDのバリデーションもしているつもりですが...

<?php

namespace App\Controllers;

use App\Models\NewsModel;
use CodeIgniter\Exceptions\PageNotFoundException;
use CodeIgniter\HTTP\ResponseInterface;

class News3 extends BaseController
{
    private NewsModel $model;

    /**
     * Validation Rules
     */
    private array $rules = [
        'title' => 'required|min_length[3]|max_length[255]',
        'body'  => 'required|min_length[10]|max_length[5000]',
    ];

    public function __construct()
    {
        $this->model = model(NewsModel::class);
    }

    // ...

    /**
     * @return ResponseInterface|string
     */
    public function update()
    {
        $id = $this->request->getPost('id');

        // Adds validation rule for id.
        $rules = array_merge($this->rules, ['id' => 'required|is_natural_no_zero']);

        if ($this->validate($rules)) {
            $title = $this->request->getVar('title');
            $slug  = url_title($title, '-', true);

            $data = [
                'title' => $title,
                'slug'  => $slug,
                'body'  => $this->request->getVar('body'),
            ];
            $this->model->update($id, $data);

            return $this->response->redirect(site_url('news3/' . $slug));
        }

        return $this->edit($id);
    }
}