Skip to content

[BUG] $_REQUEST used unsanitized in Report.php — potential injection and GLPI 11 incompatibility #46

@giovanny07

Description

@giovanny07

Plugin version: 3.2.6
GLPI version: 11.x
Severity: Medium
File: src/Report.php ~lines 2026–2034

Description

Report.php reads $_REQUEST['sort'] and $_REQUEST['order'] directly without sanitization and iterates over the entire $_REQUEST superglobal:

// src/Report.php ~line 2026
if (isset($_REQUEST['sort']) && $_REQUEST['sort'] == $columnname) {
    if (isset($_REQUEST['order']) && $_REQUEST['order'] == 'ASC') {
        // ...
    }
}

foreach ($_REQUEST as $name => $value) {
    // builds query parameters
}

Two problems:

  1. Unsanitized input: Using $_REQUEST directly without going through Sanitize::sanitize() or GLPI's \Glpi\Http\Request abstraction is unsafe and produces a deprecation warning in GLPI 11, which enforces input sanitization through its middleware layer.

  2. $_REQUEST iteration: Iterating over the entire $_REQUEST superglobal (which merges $_GET, $_POST, and $_COOKIE) to build query parameters is unpredictable and may expose internal cookie values or allow parameter injection.

Proposed fix

Replace direct $_REQUEST access with explicit, typed reads using GLPI's sanitization:

// src/Report.php

// ❌ Before
if (isset($_REQUEST['sort']) && $_REQUEST['sort'] == $columnname) {
    if (isset($_REQUEST['order']) && $_REQUEST['order'] == 'ASC') {
        $order = 'ASC';
    } else {
        $order = 'DESC';
    }
}

foreach ($_REQUEST as $name => $value) {
    echo Html::hidden($name, ['value' => $value]);
}

// ✅ After
$allowedSortOrders = ['ASC', 'DESC'];

$currentSort  = isset($_GET['sort'])  ? Sanitize::sanitize($_GET['sort'])  : '';
$currentOrder = isset($_GET['order']) ? Sanitize::sanitize($_GET['order']) : 'ASC';

if (!in_array($currentOrder, $allowedSortOrders, true)) {
    $currentOrder = 'ASC';
}

if ($currentSort === $columnname) {
    $order = $currentOrder === 'ASC' ? 'DESC' : 'ASC';
}

// For preserving params across pagination — use an explicit allowlist
$allowedParams = ['sort', 'order', 'start', 'users_id', 'begin', 'end'];
foreach ($allowedParams as $param) {
    if (isset($_GET[$param])) {
        echo Html::hidden($param, ['value' => Sanitize::sanitize($_GET[$param])]);
    }
}

Additional context

  • GLPI 11 strongly recommends using \Glpi\Http\Request (PSR-7 compatible) instead of superglobals. For a full GLPI 11 compliance upgrade, consider injecting the request object.
  • The foreach ($_REQUEST as ...) pattern is particularly dangerous because $_REQUEST includes cookies, which an attacker could control.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions