Skip to content

Commit 26f8f85

Browse files
LexioJLexioJ
authored
Merge pull request #4 from LexioJ:fix/1.3.1-security-hardening
Release v1.3.1 - Security Hardening
2 parents f5c7b9a + 16e027f commit 26f8f85

File tree

5 files changed

+164
-114
lines changed

5 files changed

+164
-114
lines changed

‎CHANGELOG.md‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
---
99

10+
## [1.3.1] - 2025-11-09
11+
12+
### 🔒 Security Hardening
13+
14+
This release includes important security enhancements to strengthen the application's defenses.
15+
16+
### Changed
17+
- Replaced direct cURL usage with Nextcloud's IClientService for proper SSL certificate verification
18+
- Enhanced input validation and sanitization for OQL query parameters
19+
- Improved data validation for numeric identifiers
20+
21+
### Security
22+
- Resolved issues related to network communication security
23+
- Strengthened protection against malicious input in database queries
24+
25+
---
26+
1027
## [1.3.0] - 2025-11-08
1128

1229
### ✨ Major New Feature: Agent Notifications System

‎appinfo/info.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ Seamlessly connect your Nextcloud collaboration platform with iTop IT Service Ma
7575
7676
#### Start streamlining your ITSM workflow - Connect iTop with Nextcloud and experience unified IT service management!]]></description>
7777

78-
<version>1.3.0</version>
78+
<version>1.3.1</version>
7979
<licence>AGPL-3.0-or-later</licence>
8080

8181
<author>iTop Integration Team</author>

‎lib/Controller/ConfigController.php‎

Lines changed: 76 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use OCP\AppFramework\Controller;
2020
use OCP\AppFramework\Http;
2121
use OCP\AppFramework\Http\DataResponse;
22+
use OCP\Http\Client\IClientService;
2223
use OCP\IConfig;
2324
use OCP\IL10N;
2425
use OCP\IRequest;
@@ -39,6 +40,7 @@ public function __construct(
3940
private CacheService $cacheService,
4041
private LoggerInterface $logger,
4142
private IAppManager $appManager,
43+
private IClientService $clientService,
4244
private ?string $userId
4345
) {
4446
parent::__construct($appName, $request);
@@ -269,26 +271,20 @@ public function getUserInfo(): DataResponse {
269271
])
270272
];
271273

272-
$ch = curl_init();
273-
curl_setopt($ch, CURLOPT_URL, $apiUrl);
274-
curl_setopt($ch, CURLOPT_POST, true);
275-
curl_setopt($ch, CURLOPT_POSTFIELDS, http_build_query($postData));
276-
curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
277-
curl_setopt($ch, CURLOPT_TIMEOUT, 15);
278-
curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, false);
279-
curl_setopt($ch, CURLOPT_HTTPHEADER, [
280-
'Content-Type: application/x-www-form-urlencoded',
281-
'Auth-Token: ' . $applicationToken,
282-
'User-Agent: Nextcloud-iTop-Integration/1.0'
283-
]);
284-
285-
$result = curl_exec($ch);
286-
$httpCode = curl_getinfo($ch, CURLINFO_HTTP_CODE);
287-
$error = curl_error($ch);
288-
curl_close($ch);
289-
290-
if ($result === false || !empty($error)) {
291-
return new DataResponse(['error' => $this->l10n->t('Connection failed: %s', [$error ?: $this->l10n->t('Unknown error')])], Http::STATUS_SERVICE_UNAVAILABLE);
274+
try {
275+
$client = $this->clientService->newClient();
276+
$response = $client->post($apiUrl, [
277+
'body' => http_build_query($postData),
278+
'headers' => [
279+
'Content-Type' => 'application/x-www-form-urlencoded',
280+
'Auth-Token' => $applicationToken,
281+
'User-Agent' => 'Nextcloud-iTop-Integration/1.0'
282+
],
283+
'timeout' => 15,
284+
]);
285+
$result = $response->getBody();
286+
} catch (\Exception $e) {
287+
return new DataResponse(['error' => $this->l10n->t('Connection failed: %s', [$e->getMessage()])], Http::STATUS_SERVICE_UNAVAILABLE);
292288
}
293289

294290
$responseData = json_decode($result, true);
@@ -412,30 +408,24 @@ public function testApplicationToken(string $token = ''): DataResponse {
412408
])
413409
];
414410

415-
$ch = curl_init();
416-
curl_setopt($ch, CURLOPT_URL, $apiUrl);
417-
curl_setopt($ch, CURLOPT_POST, true);
418-
curl_setopt($ch, CURLOPT_POSTFIELDS, http_build_query($postData));
419-
curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
420-
curl_setopt($ch, CURLOPT_TIMEOUT, 15);
421-
curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, false);
422-
curl_setopt($ch, CURLOPT_HTTPHEADER, [
423-
'Content-Type: application/x-www-form-urlencoded',
424-
'Auth-Token: ' . $token,
425-
'User-Agent: Nextcloud-iTop-Integration/1.0'
426-
]);
427-
428-
$result = curl_exec($ch);
429-
$httpCode = curl_getinfo($ch, CURLINFO_HTTP_CODE);
430-
$error = curl_error($ch);
431-
curl_close($ch);
432-
433-
$this->logger->info('iTop application token test - Method 1 (Auth-Token header) response: ' . $result, ['app' => Application::APP_ID]);
411+
try {
412+
$client = $this->clientService->newClient();
413+
$response = $client->post($apiUrl, [
414+
'body' => http_build_query($postData),
415+
'headers' => [
416+
'Content-Type' => 'application/x-www-form-urlencoded',
417+
'Auth-Token' => $token,
418+
'User-Agent' => 'Nextcloud-iTop-Integration/1.0'
419+
],
420+
'timeout' => 15,
421+
]);
422+
$result = $response->getBody();
434423

435-
if ($result === false || !empty($error)) {
424+
$this->logger->info('iTop application token test response: ' . $result, ['app' => Application::APP_ID]);
425+
} catch (\Exception $e) {
436426
return new DataResponse([
437427
'status' => 'error',
438-
'message' => $this->l10n->t('Connection failed: %s', [$error ?: $this->l10n->t('Unknown error')])
428+
'message' => $this->l10n->t('Connection failed: %s', [$e->getMessage()])
439429
]);
440430
}
441431

@@ -532,37 +522,23 @@ public function testAdminConnection(string $url = ''): DataResponse {
532522
'operation' => 'core/check_credentials'
533523
])
534524
];
535-
536-
$ch = curl_init();
537-
curl_setopt($ch, CURLOPT_URL, $apiUrl);
538-
curl_setopt($ch, CURLOPT_POST, true);
539-
curl_setopt($ch, CURLOPT_POSTFIELDS, http_build_query($postData));
540-
curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
541-
curl_setopt($ch, CURLOPT_TIMEOUT, 15);
542-
curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, false);
543-
curl_setopt($ch, CURLOPT_HTTPHEADER, [
544-
'Content-Type: application/x-www-form-urlencoded',
545-
'User-Agent: Nextcloud-iTop-Integration/1.0'
546-
]);
547-
548-
$result = curl_exec($ch);
549-
$httpCode = curl_getinfo($ch, CURLINFO_HTTP_CODE);
550-
$error = curl_error($ch);
551-
curl_close($ch);
552-
553-
if ($result === false || !empty($error)) {
554-
return new DataResponse([
555-
'status' => 'error',
556-
'message' => $this->l10n->t('Connection failed: %s', [$error ?: $this->l10n->t('Unknown error')]),
557-
'details' => ['url' => $testUrl, 'api_url' => $apiUrl]
558-
]);
559-
}
560525

561-
if ($httpCode !== 200) {
526+
try {
527+
$client = $this->clientService->newClient();
528+
$response = $client->post($apiUrl, [
529+
'body' => http_build_query($postData),
530+
'headers' => [
531+
'Content-Type' => 'application/x-www-form-urlencoded',
532+
'User-Agent' => 'Nextcloud-iTop-Integration/1.0'
533+
],
534+
'timeout' => 15,
535+
]);
536+
$result = $response->getBody();
537+
} catch (\Exception $e) {
562538
return new DataResponse([
563539
'status' => 'error',
564-
'message' => $this->l10n->t('iTop API endpoint returned HTTP %d', [$httpCode]),
565-
'details' => ['http_code' => $httpCode, 'url' => $testUrl, 'api_url' => $apiUrl]
540+
'message' => $this->l10n->t('Connection failed: %s', [$e->getMessage()]),
541+
'details' => ['url' => $testUrl, 'api_url' => $apiUrl]
566542
]);
567543
}
568544

@@ -1207,30 +1183,24 @@ private function validatePersonalTokenAndExtractPersonId(string $personalToken):
12071183
])
12081184
];
12091185

1210-
$ch = curl_init();
1211-
curl_setopt($ch, CURLOPT_URL, $apiUrl);
1212-
curl_setopt($ch, CURLOPT_POST, true);
1213-
curl_setopt($ch, CURLOPT_POSTFIELDS, http_build_query($postData));
1214-
curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
1215-
curl_setopt($ch, CURLOPT_TIMEOUT, 15);
1216-
curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, false);
1217-
curl_setopt($ch, CURLOPT_HTTPHEADER, [
1218-
'Content-Type: application/x-www-form-urlencoded',
1219-
'Auth-Token: ' . $personalToken,
1220-
'User-Agent: Nextcloud-iTop-Integration/1.0'
1221-
]);
1222-
1223-
$result = curl_exec($ch);
1224-
$httpCode = curl_getinfo($ch, CURLINFO_HTTP_CODE);
1225-
$error = curl_error($ch);
1226-
curl_close($ch);
1227-
1228-
if ($result === false || !empty($error)) {
1186+
try {
1187+
$client = $this->clientService->newClient();
1188+
$response = $client->post($apiUrl, [
1189+
'body' => http_build_query($postData),
1190+
'headers' => [
1191+
'Content-Type' => 'application/x-www-form-urlencoded',
1192+
'Auth-Token' => $personalToken,
1193+
'User-Agent' => 'Nextcloud-iTop-Integration/1.0'
1194+
],
1195+
'timeout' => 15,
1196+
]);
1197+
$result = $response->getBody();
1198+
} catch (\Exception $e) {
12291199
return [
12301200
'success' => false,
12311201
'person_id' => null,
12321202
'user_info' => null,
1233-
'error' => $this->l10n->t('Connection failed: %s', [$error ?: $this->l10n->t('Unknown error')])
1203+
'error' => $this->l10n->t('Connection failed: %s', [$e->getMessage()])
12341204
];
12351205
}
12361206

@@ -1297,6 +1267,11 @@ private function validatePersonalTokenAndExtractPersonId(string $personalToken):
12971267
$applicationToken = $this->crypto->decrypt($encryptedAppToken);
12981268

12991269
// Query User class using application token
1270+
// Validate personId to prevent OQL injection
1271+
if (!is_numeric($personId) || $personId < 0) {
1272+
throw new \InvalidArgumentException('Invalid person ID');
1273+
}
1274+
$personId = (int)$personId;
13001275
$getUserData = [
13011276
'json_data' => json_encode([
13021277
'operation' => 'core/get',
@@ -1305,23 +1280,19 @@ private function validatePersonalTokenAndExtractPersonId(string $personalToken):
13051280
'output_fields' => 'id,login,finalclass'
13061281
])
13071282
];
1308-
1309-
$ch2 = curl_init();
1310-
curl_setopt($ch2, CURLOPT_URL, $apiUrl);
1311-
curl_setopt($ch2, CURLOPT_POST, true);
1312-
curl_setopt($ch2, CURLOPT_POSTFIELDS, http_build_query($getUserData));
1313-
curl_setopt($ch2, CURLOPT_RETURNTRANSFER, true);
1314-
curl_setopt($ch2, CURLOPT_TIMEOUT, 15);
1315-
curl_setopt($ch2, CURLOPT_SSL_VERIFYPEER, false);
1316-
curl_setopt($ch2, CURLOPT_HTTPHEADER, [
1317-
'Content-Type: application/x-www-form-urlencoded',
1318-
'Auth-Token: ' . $applicationToken,
1319-
'User-Agent: Nextcloud-iTop-Integration/1.0'
1283+
1284+
$client = $this->clientService->newClient();
1285+
$response = $client->post($apiUrl, [
1286+
'body' => http_build_query($getUserData),
1287+
'headers' => [
1288+
'Content-Type' => 'application/x-www-form-urlencoded',
1289+
'Auth-Token' => $applicationToken,
1290+
'User-Agent' => 'Nextcloud-iTop-Integration/1.0'
1291+
],
1292+
'timeout' => 15,
13201293
]);
1321-
1322-
$userResult = curl_exec($ch2);
1323-
curl_close($ch2);
1324-
1294+
$userResult = $response->getBody();
1295+
13251296
$userData = json_decode($userResult, true);
13261297
if (isset($userData['objects']) && !empty($userData['objects'])) {
13271298
$userObject = reset($userData['objects']);

‎lib/Service/ItopAPIService.php‎

Lines changed: 63 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,57 @@ private function getPersonId(string $userId): ?string {
122122
return $personId !== '' ? $personId : null;
123123
}
124124

125+
/**
126+
* Escape a string value for use in OQL queries
127+
* Protects against OQL injection by properly escaping special characters
128+
*
129+
* @param string $value Value to escape
130+
* @return string Escaped value safe for OQL
131+
*/
132+
private function escapeOQLString(string $value): string {
133+
// Escape backslashes first to prevent double-escaping
134+
$value = str_replace('\\', '\\\\', $value);
135+
// Escape single quotes
136+
$value = str_replace("'", "\\'", $value);
137+
// Escape double quotes
138+
$value = str_replace('"', '\\"', $value);
139+
return $value;
140+
}
141+
142+
/**
143+
* Validate and sanitize a numeric ID for use in OQL queries
144+
* Throws exception if ID is not numeric to prevent injection
145+
*
146+
* @param mixed $id ID to validate
147+
* @return int Validated numeric ID
148+
* @throws \InvalidArgumentException If ID is not numeric
149+
*/
150+
private function validateNumericId($id): int {
151+
if (!is_numeric($id) || $id < 0) {
152+
throw new \InvalidArgumentException('Invalid ID: must be a positive integer');
153+
}
154+
return (int)$id;
155+
}
156+
157+
/**
158+
* Validate class name against whitelist to prevent injection
159+
*
160+
* @param string $class Class name to validate
161+
* @return string Validated class name
162+
* @throws \InvalidArgumentException If class is not in whitelist
163+
*/
164+
private function validateClassName(string $class): string {
165+
$allowedClasses = ['UserRequest', 'Incident', 'Person', 'User', 'Team', 'lnkContactToTicket',
166+
'lnkPersonToTeam', 'Change', 'PC', 'Phone', 'IPPhone', 'MobilePhone', 'Tablet',
167+
'Printer', 'Peripheral', 'PCSoftware', 'OtherSoftware', 'WebApplication', 'Software',
168+
'SoftwareInstance', 'lnkContactToFunctionalCI'];
169+
170+
if (!in_array($class, $allowedClasses, true)) {
171+
throw new \InvalidArgumentException('Invalid class name: ' . $class);
172+
}
173+
return $class;
174+
}
175+
125176
/**
126177
* Get tickets created by the current user (UserRequest + Incident)
127178
*
@@ -153,14 +204,16 @@ public function getUserCreatedTickets(string $userId, ?string $since = null, ?in
153204

154205
// Get person_id for more precise queries
155206
$personId = $this->config->getUserValue($userId, Application::APP_ID, 'person_id', '');
156-
207+
157208
// Query UserRequest tickets where user is caller OR contact
158209
// Note: ORDER BY in complex queries with subqueries may not work, so we sort in PHP
159210
if ($personId) {
160-
$userRequestQuery = "SELECT UserRequest WHERE (caller_id = '$personId' OR id IN (SELECT UserRequest JOIN lnkContactToTicket ON lnkContactToTicket.ticket_id = UserRequest.id WHERE lnkContactToTicket.contact_id = '$personId')) AND status != 'closed'";
211+
$personId = $this->validateNumericId($personId);
212+
$userRequestQuery = "SELECT UserRequest WHERE (caller_id = $personId OR id IN (SELECT UserRequest JOIN lnkContactToTicket ON lnkContactToTicket.ticket_id = UserRequest.id WHERE lnkContactToTicket.contact_id = $personId)) AND status != 'closed'";
161213
} else {
162214
// Fallback to name-based query if no person_id
163-
$userRequestQuery = "SELECT UserRequest WHERE caller_id_friendlyname = '$fullName' AND status != 'closed'";
215+
$escapedFullName = $this->escapeOQLString($fullName);
216+
$userRequestQuery = "SELECT UserRequest WHERE caller_id_friendlyname = '$escapedFullName' AND status != 'closed'";
164217
}
165218
$userRequestParams = [
166219
'operation' => 'core/get',
@@ -210,10 +263,11 @@ public function getUserCreatedTickets(string $userId, ?string $since = null, ?in
210263

211264
// Query Incident tickets where user is caller OR contact
212265
if ($personId) {
213-
$incidentQuery = "SELECT Incident WHERE (caller_id = '$personId' OR id IN (SELECT Incident JOIN lnkContactToTicket ON lnkContactToTicket.ticket_id = Incident.id WHERE lnkContactToTicket.contact_id = '$personId')) AND status != 'closed'";
266+
$incidentQuery = "SELECT Incident WHERE (caller_id = $personId OR id IN (SELECT Incident JOIN lnkContactToTicket ON lnkContactToTicket.ticket_id = Incident.id WHERE lnkContactToTicket.contact_id = $personId)) AND status != 'closed'";
214267
} else {
215268
// Fallback to name-based query if no person_id
216-
$incidentQuery = "SELECT Incident WHERE caller_id_friendlyname = '$fullName' AND status != 'closed'";
269+
$escapedFullName = $this->escapeOQLString($fullName);
270+
$incidentQuery = "SELECT Incident WHERE caller_id_friendlyname = '$escapedFullName' AND status != 'closed'";
217271
}
218272
$incidentParams = [
219273
'operation' => 'core/get',
@@ -339,7 +393,8 @@ public function getUserCreatedTicketsCount(string $userId): array {
339393
$requestCount = 0;
340394

341395
// Query UserRequest tickets
342-
$userRequestQuery = "SELECT UserRequest WHERE caller_id_friendlyname = '$fullName' AND status != 'closed'";
396+
$escapedFullName = $this->escapeOQLString($fullName);
397+
$userRequestQuery = "SELECT UserRequest WHERE caller_id_friendlyname = '$escapedFullName' AND status != 'closed'";
343398
$userRequestParams = [
344399
'operation' => 'core/get',
345400
'class' => 'UserRequest',
@@ -351,9 +406,9 @@ public function getUserCreatedTicketsCount(string $userId): array {
351406
if (isset($userRequestResult['objects'])) {
352407
$requestCount = count($userRequestResult['objects']);
353408
}
354-
409+
355410
// Query Incident tickets
356-
$incidentQuery = "SELECT Incident WHERE caller_id_friendlyname = '$fullName' AND status != 'closed'";
411+
$incidentQuery = "SELECT Incident WHERE caller_id_friendlyname = '$escapedFullName' AND status != 'closed'";
357412
$incidentParams = [
358413
'operation' => 'core/get',
359414
'class' => 'Incident',

0 commit comments

Comments
 (0)