Security: enforce CSRF token validation on gradebook evaluation forms

[AngelFQC]

Problem

The gradebook evaluation add (gradebook_add_eval.php) and edit (gradebook_edit_eval.php) endpoints are state-changing POST forms built on EvalForm/FormValidator. FormValidator does not inject or verify an anti-CSRF token by itself, so these forms processed forged cross-site submissions using only the victim's session cookie.

Fix

Apply the standard Chamilo legacy CSRF idiom (the same one already used in exercise/tests_category.php and group/settings.php):

• On submit, validate Security::check_token('post') before any persistence; reject with api_not_allowed(true) if the token is missing/invalid, and Security::clear_token() on success.
• On the display path, emit a fresh token via Security::get_token() + a hidden sec_token element ($form->addElement('hidden', 'sec_token') + setConstants). get_token() is only called on the display path, never before check_token() on the processing request, so it never overwrites the session token being verified.

Invariant now enforced

A gradebook evaluation can only be created or modified by a request that carries the per-session sec_token issued when the form was rendered; cross-site forged POSTs are rejected.

OWASP control

A01:2021 – Broken Access Control (CWE-352, Cross-Site Request Forgery).

⚠ Scope / follow-up

The advisory describes the gradebook module's add/edit/delete endpoints generally. This PR covers the two evaluation form endpoints that are concretely named and share the identical EvalForm pattern. The same missing-CSRF pattern very likely affects sibling gradebook endpoints (gradebook_edit_result.php, gradebook_add_result.php, gradebook_add_user.php, gradebook_edit_all.php) and the deletion actions dispatched through gradebook/index.php action parameters. Those were left out of this minimal change because they have heterogeneous structures and deserve individual review; they should receive the same token idiom as a follow-up.

Refs GHSA-vjmr-7vxh-wpg2

🤖 Generated with Claude Code

[AngelFQC]

Cerrando este PR: tras revisarlo a fondo es un falso positivo en 2.0 y además el cambio rompía los formularios.

En 2.0 el formulario de evaluación usa EvalForm extends FormValidator, que llama $this->protect() en su constructor (gradebook/lib/fe/evalform.class.php:82). protect() (inc/lib/pear/HTML/QuickForm.php:258) inyecta el hidden protect_token y registra el token en el form; y QuickForm::validate() (línea ~1513, no sobreescrito por FormValidator ni por EvalForm) valida automáticamente ese token con Security::check_token('form', $this) y hace clear_token(), redirigiendo con "Not validated" si falla.

Es decir, gradebook_add_eval.php y gradebook_edit_eval.php ya están protegidos contra CSRF por el propio framework de formularios.

El check_token('post') que añadí encima rompía el flujo: validate() ya hace clear_token(), así que mi comprobación posterior leía un token de sesión vacío y siempre fallaba; y mi get_token() en la ruta de display regeneraba el token de sesión invalidando el protect_token del form.

GHSA-vjmr-7vxh-wpg2 no aplica a 2.0 para estos endpoints. Si existe un vector real, sería en acciones de borrado del gradebook por GET/action (fuera de EvalForm), que debe revisarse por separado.