Skip to content

Commit a6ce8fb

Browse files
committed
Security: Replace $_REQUEST with HttpFoundation\Request class and remove XSS in assign, issued, and issued_all pages
See advisory GHSA-cchj-3qmf-82j5
1 parent 74e4fa2 commit a6ce8fb

File tree

3 files changed

+45
-13
lines changed

3 files changed

+45
-13
lines changed

main/badge/assign.php

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
use Chamilo\CoreBundle\Entity\Skill;
55
use Skill as SkillManager;
6+
use Symfony\Component\HttpFoundation\Request as HttpRequest;
67

78
/**
89
* Page for assign skills to a user.
@@ -11,7 +12,12 @@
1112
*/
1213
require_once __DIR__.'/../inc/global.inc.php';
1314

14-
$userId = isset($_REQUEST['user']) ? (int) $_REQUEST['user'] : 0;
15+
$httpRequest = HttpRequest::createFromGlobals();
16+
17+
$userId = $httpRequest->query->getInt(
18+
'user',
19+
$httpRequest->request->getInt('user')
20+
);
1521

1622
if (empty($userId)) {
1723
api_not_allowed(true);
@@ -53,11 +59,25 @@
5359
$skillsOptions[$skill['data']['id']] = $skill['data']['name'];
5460
}
5561
}
56-
$skillIdFromGet = isset($_REQUEST['id']) ? (int) $_REQUEST['id'] : 0;
57-
$currentValue = isset($_REQUEST['current_value']) ? (int) $_REQUEST['current_value'] : 0;
58-
$currentLevel = isset($_REQUEST['current']) ? (int) str_replace('sub_skill_id_', '', $_REQUEST['current']) : 0;
62+
$skillIdFromGet = $httpRequest->query->getInt(
63+
'id',
64+
$httpRequest->request->getInt('id')
65+
);
66+
$currentValue = $httpRequest->query->getInt(
67+
'current_value',
68+
$httpRequest->request->getInt('current_value')
69+
);
70+
$currentLevel = $httpRequest->query->get(
71+
'current',
72+
$httpRequest->request->get('current', '')
73+
);
74+
$currentLevel = (int) str_replace('sub_skill_id_', '', $currentLevel);
5975

60-
$subSkillList = isset($_REQUEST['sub_skill_list']) ? explode(',', $_REQUEST['sub_skill_list']) : [];
76+
$subSkillList = $httpRequest->query->get(
77+
'sub_skill_list',
78+
$httpRequest->request->get('sub_skill_list', '')
79+
);
80+
$subSkillList = explode(',', $subSkillList);
6181
$subSkillList = array_unique($subSkillList);
6282

6383
if (!empty($subSkillList)) {
@@ -94,7 +114,10 @@
94114
}
95115
}
96116

97-
$skillId = isset($_REQUEST['id']) ? (int) $_REQUEST['id'] : key($skillsOptions);
117+
$skillId = $httpRequest->query->getInt(
118+
'id',
119+
$httpRequest->request->getInt('id', key($skillsOptions))
120+
);
98121
$skill = $skillRepo->find($skillId);
99122
$profile = false;
100123
if ($skill) {
@@ -234,15 +257,15 @@
234257
//$form->addRule('acquired_level', get_lang('ThisFieldIsRequired'), 'required');
235258
}
236259

237-
$form->addTextarea('argumentation', get_lang('Argumentation'), ['rows' => 6]);
238-
$form->addRule('argumentation', get_lang('ThisFieldIsRequired'), 'required');
260+
$form->addTextarea('argumentation', get_lang('Argumentation'), ['rows' => 6], true);
239261
$form->addRule(
240262
'argumentation',
241263
sprintf(get_lang('ThisTextShouldBeAtLeastXCharsLong'), 10),
242264
'mintext',
243265
10
244266
);
245267
$form->applyFilter('argumentation', 'trim');
268+
$form->applyFilter('argumentation', 'html_filter');
246269
$form->addButtonSave(get_lang('Save'));
247270
$form->setDefaults($formDefaultValues);
248271

main/badge/issued.php

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
use Chamilo\CoreBundle\Entity\SkillRelUser;
55
use Chamilo\CoreBundle\Entity\SkillRelUserComment;
66
use SkillRelUser as SkillRelUserManager;
7+
use Symfony\Component\HttpFoundation\Request as HttpRequest;
78

89
/**
910
* Show information about the issued badge.
@@ -13,7 +14,12 @@
1314
*/
1415
require_once __DIR__.'/../inc/global.inc.php';
1516

16-
$issue = isset($_REQUEST['issue']) ? (int) $_REQUEST['issue'] : 0;
17+
$httpRequest = HttpRequest::createFromGlobals();
18+
19+
$issue = $httpRequest->query->getInt(
20+
'issue',
21+
$httpRequest->request->getInt('issue')
22+
);
1723

1824
if (empty($issue)) {
1925
api_not_allowed(true);
@@ -103,7 +109,7 @@
103109
'acquired_level' => $currentSkillLevel,
104110
'argumentation_author_id' => $skillIssue->getArgumentationAuthorId(),
105111
'argumentation_author_name' => $author['complete_name'],
106-
'argumentation' => $skillIssue->getArgumentation(),
112+
'argumentation' => Security::remove_XSS($skillIssue->getArgumentation()),
107113
'source_name' => $skillIssue->getSourceName(),
108114
'user_id' => $skillIssue->getUser()->getId(),
109115
'user_complete_name' => UserManager::formatUserFullName($skillIssue->getUser()),

main/badge/issued_all.php

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
use Chamilo\CoreBundle\Entity\SkillRelUser;
55
use Chamilo\CoreBundle\Entity\SkillRelUserComment;
66
use SkillRelUser as SkillRelUserManager;
7+
use Symfony\Component\HttpFoundation\Request as HttpRequest;
78

89
/**
910
* Show information about all issued badges with same skill by user.
@@ -12,8 +13,10 @@
1213
*/
1314
require_once __DIR__.'/../inc/global.inc.php';
1415

15-
$userId = isset($_GET['user']) ? (int) $_GET['user'] : 0;
16-
$skillId = isset($_GET['skill']) ? (int) $_GET['skill'] : 0;
16+
$httpRequest = HttpRequest::createFromGlobals();
17+
18+
$userId = $httpRequest->query->getInt('user');
19+
$skillId = $httpRequest->query->getInt('skill');
1720

1821
if (!$userId || !$skillId) {
1922
api_not_allowed(true);
@@ -90,7 +93,7 @@
9093
$argumentationAuthor['firstname'],
9194
$argumentationAuthor['lastname']
9295
),
93-
'argumentation' => $skillIssue->getArgumentation(),
96+
'argumentation' => Security::remove_XSS($skillIssue->getArgumentation()),
9497
'source_name' => $skillIssue->getSourceName(),
9598
'user_id' => $skillIssue->getUser()->getId(),
9699
'user_complete_name' => UserManager::formatUserFullName($skillIssue->getUser()),

0 commit comments

Comments
 (0)