Skip to content

Commit f91c289

Browse files
monta990monta990
authored
Security Fixes
1 parent adf48f5 commit f91c289

8 files changed

Lines changed: 41 additions & 36 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,18 @@
33
All notable changes to this project will be documented in this file.
44
Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
55

6+
## [1.3.7] — 2026-05-03
7+
8+
### Security
9+
- **Credential redaction in debug logs** — GWN OAuth token request URL and token response are no longer logged verbatim. Debug output now shows only `appID` and `expires_in`, preventing `client_secret` and `access_token` from appearing in `files/_log/gdmsintegration.log` even when verbose mode is enabled. (OWASP A02/A09)
10+
- **Advisory lock queries use escaped identifiers** — `GET_LOCK`/`RELEASE_LOCK` raw SQL queries in the sync engine now call `$DB->escape()` on the lock name, following defense-in-depth for all raw query parameters. (OWASP A03)
11+
- **Webhook signature mismatch returns 204** — failed HMAC verification no longer returns HTTP 403 (which confirmed the endpoint existed and the signature was checked). Now returns `204 No Content`, removing the oracle useful for probing or brute-forcing. (OWASP A07)
12+
- **Webhook GET handler removed** — the unauthenticated `GET` health-check response that disclosed plugin name and endpoint path has been removed. Non-POST requests now receive `405` with no body. (OWASP A05)
13+
- **Webhook payload logging scoped** — log line now records only `entity`, `mac`, and `status` rather than up to 500 chars of raw JSON payload. (OWASP A09)
14+
- **Firmware endpoint requires READ right** — `firmware.ajax.php` read actions (`check`, `check_all`) now require `Session::checkRight('config', READ)` instead of a plain session check, preventing any authenticated GLPI user from querying firmware status of all devices. (OWASP A01)
15+
16+
---
17+
618
## [1.3.6] — 2026-04-29
719

820
### Added

‎LICENSE‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -652,7 +652,7 @@ Also add information on how to contact you by electronic and paper mail.
652652
If the program does terminal interaction, make it output a short
653653
notice like this when it starts in an interactive mode:
654654

655-
<program> Copyright (C) <year> <name of author>
655+
GDMSIntegration Copyright (C) 2026 Edwin Elias (monta990)
656656
This program comes with ABSOLUTELY NO WARRANTY; for details type `show w'.
657657
This is free software, and you are welcome to redistribute it
658658
under certain conditions; type `show c' for details.

‎front/firmware.ajax.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
if (in_array($_action_early, ['upgrade', 'upgrade_gdms'], true)) {
2525
Session::checkRight('config', UPDATE);
2626
} else {
27-
Session::checkLoginUser();
27+
Session::checkRight('config', READ);
2828
}
2929
header('Content-Type: application/json');
3030

‎front/webhook.php‎

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,21 @@
33
* GDMS Integration — Webhook receiver
44
* Registered as stateless — called by GDMS Cloud without browser session.
55
* Uses correct Symfony exception namespaces for GLPI 11.
6+
*
7+
* Note: GDMS Cloud does not support configurable webhook secrets/signatures.
8+
* The HMAC check runs only when webhook_secret is configured (e.g. for custom
9+
* integrations or future GDMS support). Without a secret this endpoint is
10+
* open to any POST — restrict access at the network/firewall level.
611
*/
712

813
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
914
use Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException;
1015
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
1116

12-
// Allow GET for health check / manual testing
13-
if ($_SERVER['REQUEST_METHOD'] === 'GET') {
14-
header('Content-Type: application/json');
15-
echo json_encode(['status' => 'ok', 'plugin' => 'gdmsintegration', 'endpoint' => 'webhook']);
16-
return;
17-
}
18-
1917
if ($_SERVER['REQUEST_METHOD'] !== 'POST') {
20-
throw new MethodNotAllowedHttpException(['POST'], 'Only POST requests are accepted');
18+
// Silently reject non-POST — no plugin fingerprint disclosed
19+
http_response_code(405);
20+
exit;
2121
}
2222

2323
$raw = file_get_contents('php://input');
@@ -32,8 +32,9 @@
3232
$sig_header = $_SERVER['HTTP_X_GDMS_SIGNATURE'] ?? '';
3333
$expected = 'sha256=' . hash_hmac('sha256', $raw, $secret);
3434
if (!hash_equals($expected, $sig_header)) {
35-
PluginGdmsintegrationUtils::log("Webhook: signature mismatch from " . ($_SERVER['REMOTE_ADDR'] ?? ''));
36-
throw new AccessDeniedHttpException('Invalid webhook signature');
35+
// Return 204 — don't confirm endpoint or leak that signature was checked
36+
http_response_code(204);
37+
exit;
3738
}
3839
}
3940

@@ -42,26 +43,18 @@
4243
throw new BadRequestHttpException('Invalid JSON payload');
4344
}
4445

45-
PluginGdmsintegrationUtils::log(
46-
"Webhook received — entity:{$entity} | payload:" . substr(json_encode($payload), 0, 500)
47-
);
48-
49-
// Process device status change from GDMS cloud
50-
$mac = strtolower(trim($payload['mac'] ?? $payload['device_mac'] ?? $payload['deviceMac'] ?? ''));
46+
$mac = strtolower(trim($payload['mac'] ?? $payload['device_mac'] ?? $payload['deviceMac'] ?? ''));
5147
$rawStatus = strtolower($payload['status'] ?? $payload['deviceStatus'] ?? $payload['event'] ?? '');
52-
$status = in_array($rawStatus, ['offline', 'disconnect', 'down', '0']) ? 'offline' : 'online';
48+
$status = in_array($rawStatus, ['offline', 'disconnect', 'down', '0']) ? 'offline' : 'online';
5349

54-
PluginGdmsintegrationUtils::log("Webhook: {$mac} → {$status} (raw: {$rawStatus})");
50+
PluginGdmsintegrationUtils::log("Webhook: entity:{$entity} mac:{$mac} status:{$status}");
5551

5652
if (!empty($mac)) {
57-
// Update state immediately so dashboard reflects it
58-
$state = new PluginGdmsintegrationDevice();
53+
$state = new PluginGdmsintegrationDevice();
5954
$prevStatus = $state->getState($mac);
6055
$state->saveStateWithNetwork($mac, $status);
6156

62-
// Fire ticket logic on status transition
6357
if ($prevStatus !== $status) {
64-
// Load asset info for ticket creation
6558
$assetInfo = null;
6659
foreach (['NetworkEquipment', 'Phone'] as $itemtype) {
6760
$obj = new $itemtype();
@@ -76,9 +69,9 @@
7669
$deviceRow = $state->find(['mac' => $mac]);
7770
$dr = !empty($deviceRow) ? reset($deviceRow) : [];
7871
PluginGdmsintegrationSync::triggerOfflineTicket(
79-
$assetInfo['name'] ?? $mac,
72+
$assetInfo['name'] ?? $mac,
8073
$mac,
81-
$assetInfo['serial'] ?? '',
74+
$assetInfo['serial'] ?? '',
8275
$assetInfo['entities_id'] ?? $entity,
8376
$assetInfo['_itemtype'],
8477
(int)$assetInfo['id'],

‎inc/api.class.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,9 +290,9 @@ public static function gwnGetToken(array $config): string|false {
290290
'client_id' => $config['gwn_client_id'],
291291
'client_secret' => $config['gwn_client_secret'],
292292
]);
293-
PluginGdmsintegrationUtils::debug("GWN token request URL: {$url}");
293+
PluginGdmsintegrationUtils::debug("GWN token request — appID:{$config['gwn_client_id']} [credentials redacted]");
294294
$data = self::curl($url);
295-
PluginGdmsintegrationUtils::debug("GWN token response: " . json_encode($data));
295+
PluginGdmsintegrationUtils::debug("GWN token response — expires_in:" . ($data['expires_in'] ?? '?'));
296296
if ($data === false || empty($data['access_token'])) {
297297
PluginGdmsintegrationUtils::log("GWN token ERROR — appID:{$config['gwn_client_id']}");
298298
return false;

‎inc/sync.class.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -761,7 +761,7 @@ private static function createWanDownTicket(
761761
// attempt to open a WAN ticket for the same port at the same time.
762762
$lock_name = 'gdmsinteg_' . substr(md5("wan_{$reason}_{$itemtype}_{$glpi_id}_{$portSilk}"), 0, 24);
763763
$locked = 0;
764-
$lk_res = $DB->doQuery("SELECT GET_LOCK('{$lock_name}', 5) AS lk");
764+
$lk_res = $DB->doQuery("SELECT GET_LOCK('" . $DB->escape($lock_name) . "', 5) AS lk");
765765
if ($lk_res) {
766766
$lk_row = $DB->fetchAssoc($lk_res);
767767
$locked = (int)($lk_row['lk'] ?? 0);
@@ -854,7 +854,7 @@ private static function createWanDownTicket(
854854
PluginGdmsintegrationUtils::log("GDMS: WAN ticket #{$ticket_id} → {$deviceName} port {$portSilk} | entity={$entities_id}{$tech_info}");
855855
}
856856
} finally {
857-
$DB->doQuery("SELECT RELEASE_LOCK('{$lock_name}')");
857+
$DB->doQuery("SELECT RELEASE_LOCK('" . $DB->escape($lock_name) . "')");
858858
}
859859
}
860860

@@ -919,7 +919,7 @@ private static function createOfflineTicket(
919919
// same time) both pass the open-ticket check simultaneously. ──────
920920
$lock_name = 'gdmsinteg_' . substr(md5("offline_{$itemtype}_{$glpi_id}"), 0, 24);
921921
$locked = 0;
922-
$lk_res = $DB->doQuery("SELECT GET_LOCK('{$lock_name}', 5) AS lk");
922+
$lk_res = $DB->doQuery("SELECT GET_LOCK('" . $DB->escape($lock_name) . "', 5) AS lk");
923923
if ($lk_res) {
924924
$lk_row = $DB->fetchAssoc($lk_res);
925925
$locked = (int)($lk_row['lk'] ?? 0);
@@ -1059,7 +1059,7 @@ private static function createOfflineTicket(
10591059
}
10601060
} finally {
10611061
// Always release the advisory lock, even if an exception was thrown
1062-
$DB->doQuery("SELECT RELEASE_LOCK('{$lock_name}')");
1062+
$DB->doQuery("SELECT RELEASE_LOCK('" . $DB->escape($lock_name) . "')");
10631063
}
10641064
}
10651065

‎plugin.xml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@
2323
</authors>
2424
<versions>
2525
<version>
26-
<num>1.3.6</num>
27-
<compatibility>~11.0.0</compatibility>
28-
<download_url>https://github.com/monta990/gdmsintegration/releases/download/v1.3.6/gdmsintegration-1.3.6.zip</download_url>
26+
<num>1.3.7</num>
27+
<compatibility>~11.0</compatibility>
28+
<download_url>https://github.com/monta990/gdmsintegration/releases/download/v1.3.7/gdmsintegration-1.3.7.zip</download_url>
2929
</version>
3030
</versions>
3131
<langs>

‎setup.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* License: GPL v3+
99
*/
1010

11-
define('PLUGIN_GDMSINTEGRATION_VERSION', '1.3.6');
11+
define('PLUGIN_GDMSINTEGRATION_VERSION', '1.3.7');
1212
define('PLUGIN_GDMSINTEGRATION_MIN_GLPI', '11.0');
1313
define('PLUGIN_GDMSINTEGRATION_MAX_GLPI', '11.99');
1414

0 commit comments

Comments
 (0)