Fix mass assignment vulnerability in bulk edit (GHSA-49mq-h2g4-grr9)

The bulk edit function iterated over all $_POST keys without a whitelist,
allowing authenticated users to inject arbitrary database columns (e.g.,
cost_price, deleted, item_type) into the update query. This bypassed
CodeIgniter 4's $allowedFields protection since Query Builder was used
directly.

Fix: Add ALLOWED_BULK_EDIT_FIELDS constant to Item model defining the
explicit whitelist of fields that can be bulk-updated. Use this constant
in the controller instead of iterating over $_POST directly.

Fields allowed: name, category, supplier_id, cost_price, unit_price,
reorder_level, description, allow_alt_description, is_serialized

Security impact: High (CVSS 8.1) - Could allow price manipulation and
data integrity violations.
This commit is contained in:
Ollama
2026-03-07 20:09:12 +00:00
committed by jekkos
parent b2fadea44a
commit 1f55d96580
2 changed files with 18 additions and 6 deletions

View File

@@ -876,12 +876,12 @@ class Items extends Secure_Controller
$items_to_update = $this->request->getPost('item_ids');
$item_data = [];
foreach ($_POST as $key => $value) {
// This field is nullable, so treat it differently
if ($key === 'supplier_id' && $value !== '') {
$item_data[$key] = $value;
} elseif ($value !== '' && !(in_array($key, ['item_ids', 'tax_names', 'tax_percents']))) {
$item_data[$key] = $value;
foreach (Item::ALLOWED_BULK_EDIT_FIELDS as $field) {
$value = $this->request->getPost($field);
if ($field === 'supplier_id' && $value !== '') {
$item_data[$field] = $value;
} elseif ($value !== null && $value !== '') {
$item_data[$field] = $value;
}
}

View File

@@ -16,6 +16,18 @@ use stdClass;
*/
class Item extends Model
{
public const ALLOWED_BULK_EDIT_FIELDS = [
'name',
'category',
'supplier_id',
'cost_price',
'unit_price',
'reorder_level',
'description',
'allow_alt_description',
'is_serialized'
];
protected $table = 'items';
protected $primaryKey = 'item_id';
protected $useAutoIncrement = true;