Skip to content

Commit e39e832

Browse files
tsmrtsmr
committed
Add security checks
1 parent c56643a commit e39e832

7 files changed

Lines changed: 48 additions & 115 deletions

File tree

‎addressing.xml‎

Lines changed: 0 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -102,102 +102,6 @@
102102
<compatibility>~10.0</compatibility>
103103
<download_url>https://github.com/pluginsGLPI/addressing/releases/download/3.0.0-rc1/glpi-addressing-3.0.0-rc1.tar.bz2</download_url>
104104
</version>
105-
<version>
106-
<num>2.9.1</num>
107-
<compatibility>9.5</compatibility>
108-
</version>
109-
<version>
110-
<num>2.9.0</num>
111-
<compatibility>9.5</compatibility>
112-
</version>
113-
<version>
114-
<num>2.8.0</num>
115-
<compatibility>9.4</compatibility>
116-
</version>
117-
<version>
118-
<num>2.7.1</num>
119-
<compatibility>9.3</compatibility>
120-
</version>
121-
<version>
122-
<num>2.7.0</num>
123-
<compatibility>9.3</compatibility>
124-
</version>
125-
<version>
126-
<num>2.6.1</num>
127-
<compatibility>9.2</compatibility>
128-
</version>
129-
<version>
130-
<num>2.6.0</num>
131-
<compatibility>9.2</compatibility>
132-
</version>
133-
<version>
134-
<num>2.5.0</num>
135-
<compatibility>9.1.1</compatibility>
136-
</version>
137-
<version>
138-
<num>2.3.0</num>
139-
<compatibility>0.90</compatibility>
140-
</version>
141-
<version>
142-
<num>2.2.0</num>
143-
<compatibility>0.85.3</compatibility>
144-
</version>
145-
<version>
146-
<num>2.1.0</num>
147-
<compatibility>0.84</compatibility>
148-
</version>
149-
<version>
150-
<num>2.0.1</num>
151-
<compatibility>0.83.3</compatibility>
152-
</version>
153-
<version>
154-
<num>2.0.0</num>
155-
<compatibility>0.83</compatibility>
156-
</version>
157-
<version>
158-
<num>1.9.1</num>
159-
<compatibility>0.80</compatibility>
160-
</version>
161-
<version>
162-
<num>1.9.0</num>
163-
<compatibility>0.80</compatibility>
164-
</version>
165-
<version>
166-
<num>1.8.0</num>
167-
<compatibility>0.78</compatibility>
168-
</version>
169-
<version>
170-
<num>1.7.2</num>
171-
<compatibility>0.72</compatibility>
172-
</version>
173-
<version>
174-
<num>1.7.1</num>
175-
<compatibility>0.72</compatibility>
176-
</version>
177-
<version>
178-
<num>1.7.0</num>
179-
<compatibility>0.72</compatibility>
180-
</version>
181-
<version>
182-
<num>1.6</num>
183-
<compatibility>0.71</compatibility>
184-
</version>
185-
<version>
186-
<num>1.4</num>
187-
<compatibility>0.70</compatibility>
188-
</version>
189-
<version>
190-
<num>1.3</num>
191-
<compatibility>0.68</compatibility>
192-
</version>
193-
<version>
194-
<num>1.2</num>
195-
<compatibility>0.68</compatibility>
196-
</version>
197-
<version>
198-
<num>1.1</num>
199-
<compatibility>0.68</compatibility>
200-
</version>
201105
</versions>
202106
<langs>
203107
<lang>cs_CZ</lang>

‎ajax/ipcomment.php‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
// ----------------------------------------------------------------------
3333

3434

35+
use Glpi\Exception\Http\AccessDeniedHttpException;
36+
use GlpiPlugin\Addressing\Addressing;
3537
use GlpiPlugin\Addressing\IpComment;
3638
use function Safe\json_encode;
3739

@@ -40,17 +42,22 @@
4042
header("Content-Type: application/json; charset=UTF-8");
4143
Html::header_nocache();
4244

43-
if (!isset($_POST['addressing_id'])) {
44-
echo json_encode(0);
45-
}
46-
if (!isset($_POST['ipname'])) {
45+
if (!isset($_POST['addressing_id'], $_POST['ipname'])) {
4746
echo json_encode(0);
47+
return;
4848
}
4949

50-
$addressing_id = $_POST['addressing_id'];
50+
$addressing_id = (int) $_POST['addressing_id'];
5151
$ipname = $_POST['ipname'];
5252
$content = $_POST['contentC'];
5353

54+
// The global right is not entity-aware: confirm the caller may act on this
55+
// addressing range (entity perimeter) before writing an IP comment to it.
56+
$addressing = new Addressing();
57+
if ($addressing_id <= 0 || !$addressing->can($addressing_id, UPDATE)) {
58+
throw new AccessDeniedHttpException();
59+
}
60+
5461
$ipcomment = new IpComment();
5562
if ($ipcomment->getFromDBByCrit(['plugin_addressing_addressings_id' => $addressing_id, 'ipname' => $ipname])) {
5663
$ipcomment->update(['id' => $ipcomment->getID(), 'comments' => $content]);

‎ajax/ping.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,15 @@
4949

5050
$ip = $_POST['ip'];
5151
$itemtype = $_POST['itemtype'];
52-
$items_id = $_POST['items_id'];
52+
$items_id = (int) ($_POST['items_id'] ?? 0);
53+
54+
// itemtype/items_id are caller-supplied and drive the PingInfo row written below.
55+
// Reject unknown classes and any item outside the caller's entity perimeter so the
56+
// endpoint cannot be used to enumerate or pollute ping data across the entity boundary.
57+
$item = getItemForItemtype($itemtype);
58+
if (!($item instanceof CommonDBTM) || !$item->can($items_id, READ)) {
59+
throw new NotFoundHttpException();
60+
}
5361

5462
$config = new Config();
5563
$config->getFromDB('1');

‎ajax/updatepinginfo.php‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,22 +32,28 @@
3232
// ----------------------------------------------------------------------
3333

3434

35+
use Glpi\Exception\Http\AccessDeniedHttpException;
3536
use GlpiPlugin\Addressing\Addressing;
3637
use GlpiPlugin\Addressing\PingInfo;
3738

3839
Session::checkRight('plugin_addressing', UPDATE);
3940
header("Content-Type: application/json; charset=UTF-8");
4041
Html::header_nocache();
4142

42-
Session::checkLoginUser();
43-
4443
if (!isset($_POST['addressing_id'])) {
4544
echo 0;
45+
return;
4646
}
47-
$addressing_id = $_POST['addressing_id'];
48-
$old_execution = ini_set("max_execution_time", "0");
47+
$addressing_id = (int) $_POST['addressing_id'];
48+
49+
// The global right is not entity-aware: confirm the caller may act on this
50+
// addressing range (entity perimeter) before rewriting its ping information.
4951
$addressing = new Addressing();
50-
$addressing->getFromDB($addressing_id);
52+
if ($addressing_id <= 0 || !$addressing->can($addressing_id, UPDATE)) {
53+
throw new AccessDeniedHttpException();
54+
}
55+
56+
$old_execution = ini_set("max_execution_time", "0");
5157
$pingInfo = new PingInfo();
5258
$pingInfo->updateAnAddressing($addressing);
5359
ini_set("max_execution_time", $old_execution);

‎front/report.form.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@
4646
}
4747

4848
$addressing = new Addressing();
49+
// checkLoginUser() is not authorization on GLPI 11, and showReport() fetches the
50+
// record by an arbitrary id. Enforce the plugin READ right AND entity access on the
51+
// requested record before disclosing its IP report — otherwise any authenticated
52+
// user could read any entity's asset/IP inventory by incrementing the id.
53+
$addressing->check((int) ($_GET['id'] ?? 0), READ);
4954
$addressing->showReport($_GET);
5055

5156
Html::footer();

‎hook.php‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -312,9 +312,6 @@ function plugin_addressing_uninstall()
312312
$profileRight->deleteByCriteria(['name' => $right['field']]);
313313
}
314314

315-
//Remove rigth from $_SESSION['glpiactiveprofile'] if exists
316-
Profile::removeRightsFromSession();
317-
318315
Profile::removeRightsFromSession();
319316
CronTask::unregister("addressing");
320317
return true;

‎src/Report.php‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -274,8 +274,12 @@ public function displayReport(&$result, $Addressing, $values, $ping_status = [])
274274
foreach ($lines as $line) {
275275
$row_num++;
276276
$item_num = 1;
277-
$name = $line["dname"];
278-
$namep = $line["pname"];
277+
// Asset/port names are stored raw in DB and rendered through the
278+
// legacy HTMLSearchOutput::showItem(), which emits its value verbatim.
279+
// Escape here so a hostile asset/port name cannot inject markup into
280+
// the report (stored XSS across the entity/right boundary).
281+
$name = htmlspecialchars((string) $line["dname"], ENT_QUOTES, 'UTF-8');
282+
$namep = htmlspecialchars((string) $line["pname"], ENT_QUOTES, 'UTF-8');
279283
// IP
280284
if ($is_html_output) {
281285
if ($Addressing->fields["reserved_ip"] && strstr(
@@ -359,6 +363,7 @@ public function displayReport(&$result, $Addressing, $values, $ping_status = [])
359363
$user->fields["firstname"]
360364
);
361365

366+
$username = htmlspecialchars((string) $username, ENT_QUOTES, 'UTF-8');
362367
if ($user->canView()) {
363368
$output_iduser = "<a href='" . $CFG_GLPI["root_doc"] . "/front/user.form.php?id="
364369
. $line["users_id"] . "'>" . $username . "</a>";
@@ -380,11 +385,12 @@ public function displayReport(&$result, $Addressing, $values, $ping_status = [])
380385

381386
// Mac
382387
if ($line["id"]) {
388+
$mac = htmlspecialchars((string) $line["mac"], ENT_QUOTES, 'UTF-8');
383389
if ($item->canView()) {
384390
$output_mac = "<a href='" . $CFG_GLPI["root_doc"] . "/front/networkport.form.php?id="
385-
. $line["id"] . "'>" . $line["mac"] . "</a>";
391+
. $line["id"] . "'>" . $mac . "</a>";
386392
} else {
387-
$output_mac = $line["mac"];
393+
$output_mac = $mac;
388394
}
389395
if ($is_html_output) {
390396
$html_output .= $output::showItem($output_mac, $item_num, $row_num);
@@ -1011,7 +1017,7 @@ function updateFA$rand() {
10111017
if ($is_html_output) {
10121018
$html_output .= $output::showItem(
10131019
'<input type="text" id="comment' . $num . '"
1014-
value="' . $comments . '">',
1020+
value="' . htmlspecialchars($comments, ENT_QUOTES, 'UTF-8') . '">',
10151021
$item_num,
10161022
$row_num,
10171023
"style='background-color:#e0e0e0' class='center' onChange='updateFA$rand()'"

0 commit comments

Comments
 (0)