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
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions