From 7ccc92a039175634f771a2362a6971fc1cb8404e Mon Sep 17 00:00:00 2001
From: Yuri Karamian <yuri@arkhost.com>
Date: Mon, 22 Jun 2026 00:53:52 +0200
Subject: [PATCH] Harden VPSAG module: credential logging, path injection,
 suspension, signature, XSS

- Redact decrypted reseller API credentials from module logs (ArkhostVPSAG_Error).
- Validate client-controlled values before they enter upstream API URL paths
  (graph time, backup file, firewall rule id, ISO id, rDNS IP); restrict VPS ID
  to digits. Prevents path/parameter injection (IDOR).
- Suspend via Disable/Enable instead of Stop/Start, and reject state-changing
  client-area actions on non-active services (suspended client can't power on).
- Verify the provisioning callback signature with hash_equals instead of !=.
- Fix callback assigned-IP assembly: keep all extra IPv4s, join with real
  newlines (was dropping the first IP and using a literal backslash-n).
- Make callback html_entity_decode array-safe.
- Escape untrusted output: JSON_HEX_TAG-encode serverInfo for the <script>
  block, HTML-escape serverInfo/OS fields in template + error.tpl, escape
  firewall/backup values built via innerHTML.
---
 modules/servers/ArkhostVPSAG/ArkhostVPSAG.php | 68 ++++++++++++++++---
 modules/servers/ArkhostVPSAG/callback.php     | 20 ++++--
 .../template/clientarea_direct.tpl            | 52 ++++++++------
 .../servers/ArkhostVPSAG/template/error.tpl   |  2 +-
 4 files changed, 104 insertions(+), 38 deletions(-)

diff --git a/modules/servers/ArkhostVPSAG/ArkhostVPSAG.php b/modules/servers/ArkhostVPSAG/ArkhostVPSAG.php
index 9a23a5e..f9c2739 100644
--- a/modules/servers/ArkhostVPSAG/ArkhostVPSAG.php
+++ b/modules/servers/ArkhostVPSAG/ArkhostVPSAG.php
@@ -19,8 +19,24 @@ use WHMCS\Database\Capsule;
 
 function ArkhostVPSAG_GetVPSID(array $params) {
     $vpsId = $params['model']->serviceProperties->get('vpsag');
-    // Handle both VPSAG-xxx and VPS-xxx formats, return just the numeric ID
-    return str_replace(['VPSAG-', 'VPS-'], '', $vpsId);
+    // Handle both VPSAG-xxx and VPS-xxx formats, return just the numeric ID.
+    // Strip everything except digits so the value is always safe in an API path.
+    return preg_replace('/[^0-9]/', '', (string) $vpsId);
+}
+
+/**
+ * Validate a client-supplied value before it is concatenated into an upstream
+ * API URL path. Rejects path traversal and any character outside the allowed
+ * set so a client cannot reshape the request (path/parameter injection / IDOR).
+ */
+function ArkhostVPSAG_SafePathSegment($value, $pattern, $label) {
+    $value = (string) $value;
+
+    if ($value === '' || strpos($value, '..') !== false || !preg_match($pattern, $value)) {
+        throw new Exception('Invalid ' . $label);
+    }
+
+    return $value;
 }
 
 function ArkhostVPSAG_API(array $params) {
@@ -91,7 +107,8 @@ function ArkhostVPSAG_API(array $params) {
             break;
 
         case 'Graphs':
-            $url .= 'vps/' . ArkhostVPSAG_GetVPSID($params) . '/graph/' . $params['time'];
+            $time = ArkhostVPSAG_SafePathSegment($params['time'], '/^[A-Za-z0-9_-]+$/', 'time period');
+            $url .= 'vps/' . ArkhostVPSAG_GetVPSID($params) . '/graph/' . $time;
             $method .= 'GET';
             break;
 
@@ -178,6 +195,9 @@ function ArkhostVPSAG_API(array $params) {
             break;
 
         case 'Reverse DNS':
+            if (!filter_var($params['ip'] ?? '', FILTER_VALIDATE_IP)) {
+                throw new Exception('Invalid IP address');
+            }
             $url .= 'vps/' . ArkhostVPSAG_GetVPSID($params) . '/rdns/' . $params['ip'];
             $method .= 'POST';
 
@@ -215,7 +235,8 @@ function ArkhostVPSAG_API(array $params) {
             break;
 
         case 'Delete backup':
-            $url .= 'vps/' . ArkhostVPSAG_GetVPSID($params) . '/backup/' . $params['file'];
+            $file = ArkhostVPSAG_SafePathSegment($params['file'], '/^[A-Za-z0-9._-]+$/', 'backup file');
+            $url .= 'vps/' . ArkhostVPSAG_GetVPSID($params) . '/backup/' . $file;
             $method .= 'DELETE';
             break;
 
@@ -225,7 +246,8 @@ function ArkhostVPSAG_API(array $params) {
             break;
 
         case 'Restore backup':
-            $url .= 'vps/' . ArkhostVPSAG_GetVPSID($params) . '/restore/' . $params['file'];
+            $file = ArkhostVPSAG_SafePathSegment($params['file'], '/^[A-Za-z0-9._-]+$/', 'backup file');
+            $url .= 'vps/' . ArkhostVPSAG_GetVPSID($params) . '/restore/' . $file;
             $method .= 'POST';
             break;
 
@@ -248,7 +270,8 @@ function ArkhostVPSAG_API(array $params) {
             break;
 
         case 'Delete Firewall rule':
-            $url .= 'vps/' . ArkhostVPSAG_GetVPSID($params) . '/firewall/' . $params['rule_id'];
+            $ruleId = ArkhostVPSAG_SafePathSegment($params['rule_id'], '/^[0-9]+$/', 'rule id');
+            $url .= 'vps/' . ArkhostVPSAG_GetVPSID($params) . '/firewall/' . $ruleId;
             $method .= 'DELETE';
             break;
 
@@ -263,7 +286,8 @@ function ArkhostVPSAG_API(array $params) {
             break;
 
         case 'Load ISO':
-            $url .= 'vps/' . ArkhostVPSAG_GetVPSID($params) . '/iso/' . $params['iso_id'];
+            $isoId = ArkhostVPSAG_SafePathSegment($params['iso_id'], '/^[0-9]+$/', 'ISO id');
+            $url .= 'vps/' . ArkhostVPSAG_GetVPSID($params) . '/iso/' . $isoId;
             $method .= 'POST';
             break;
 
@@ -319,7 +343,17 @@ function ArkhostVPSAG_API(array $params) {
 }
 
 function ArkhostVPSAG_Error($func, $params, Exception $err) {
-    logModuleCall('ArkHost - VPSAG', $func, $params, $err->getMessage(), $err->getTraceAsString());
+    // Never write the decrypted reseller API credentials into the module log.
+    $replaceVars = array();
+    if (is_array($params)) {
+        foreach (array('serverpassword', 'serverusername', 'password') as $sensitive) {
+            if (!empty($params[$sensitive]) && is_string($params[$sensitive])) {
+                $replaceVars[] = $params[$sensitive];
+            }
+        }
+    }
+
+    logModuleCall('ArkHost - VPSAG', $func, $params, $err->getMessage(), $err->getTraceAsString(), $replaceVars);
 }
 
 function ArkhostVPSAG_MetaData() {
@@ -610,7 +644,9 @@ function ArkhostVPSAG_CreateAccount(array $params) {
 
 function ArkhostVPSAG_SuspendAccount(array $params) {
     try {
-        $params['action'] = 'Stop';
+        // Use Disable (not Stop) so the client cannot simply power the VPS back
+        // on from the client area and defeat the suspension.
+        $params['action'] = 'Disable';
         ArkhostVPSAG_API($params);
     } catch (Exception $err) {
         ArkhostVPSAG_Error(__FUNCTION__, $params, $err);
@@ -622,7 +658,8 @@ function ArkhostVPSAG_SuspendAccount(array $params) {
 
 function ArkhostVPSAG_UnsuspendAccount(array $params) {
     try {
-        $params['action'] = 'Start';
+        // Mirror of SuspendAccount: re-enable the VPS that Disable suspended.
+        $params['action'] = 'Enable';
         ArkhostVPSAG_API($params);
     } catch (Exception $err) {
         ArkhostVPSAG_Error(__FUNCTION__, $params, $err);
@@ -748,6 +785,14 @@ function ArkhostVPSAG_ClientAreaAPI(array $params) {
         $results = array('result' => 'success');
 
         if (in_array($action, $actions)) {
+            // Block state-changing actions when the service is not Active, so a
+            // suspended or terminated client cannot control the VPS.
+            $stateChanging = array('Reinstall', 'Reboot', 'Stop', 'Start', 'Hostname rDNS', 'Create backup', 'Delete backup', 'Restore backup', 'Add Firewall rules', 'Delete Firewall rule', 'Commit Firewall rules', 'Load ISO', 'Eject ISO', 'Reset root');
+            $status = isset($params['model']) ? $params['model']->domainstatus : '';
+            if (in_array($action, $stateChanging) && $status !== 'Active') {
+                throw new Exception('This action is not available while the service is not active.');
+            }
+
             foreach ($_POST as $key => $value) {
                 $params[$key] = $value;
             }
@@ -1036,6 +1081,9 @@ function ArkhostVPSAG_ClientArea(array $params) {
             'templateVariables' => array(
                 'images' => $images,
                 'serverInfo' => $serverInfo,
+                // Pre-encoded for safe embedding in a <script> block: JSON_HEX_TAG
+                // prevents a customer-set hostname from breaking out with </script>.
+                'serverInfoJson' => json_encode($serverInfo, JSON_HEX_TAG | JSON_HEX_AMP) ?: '{}',
                 'operatingSystems' => $operatingSystems,
                 'token' => generate_token('plain'),
                 '_LANG' => ArkhostVPSAG_Translation(),
diff --git a/modules/servers/ArkhostVPSAG/callback.php b/modules/servers/ArkhostVPSAG/callback.php
index 8e11dba..1036d8e 100644
--- a/modules/servers/ArkhostVPSAG/callback.php
+++ b/modules/servers/ArkhostVPSAG/callback.php
@@ -18,7 +18,9 @@ require_once ROOTDIR . DIRECTORY_SEPARATOR . 'modules' . DIRECTORY_SEPARATOR . '
 
 use WHMCS\Database\Capsule;
 
-$_POST = array_map('html_entity_decode', $_POST);
+$_POST = array_map(function ($value) {
+    return is_string($value) ? html_entity_decode($value) : $value;
+}, $_POST);
 
 // Look for the service by VPSAG ID in service properties
 $serviceProperty = Capsule::table('tblservice_properties')
@@ -57,7 +59,7 @@ foreach ($_POST as $key => $value) {
 $rawSig .= hash('sha512', decrypt($server->password));
 $signature = hash('sha256', $rawSig);
 
-if ($_POST['sig'] != $signature) {
+if (!hash_equals($signature, (string) ($_POST['sig'] ?? ''))) {
     http_response_code(403);
     exit('Invalid signature');
 }
@@ -79,15 +81,21 @@ $_POST['ips'] = array_map(function ($ip) {
 
 $mainIP = $_POST['mainip'];
 
-$_POST['ips'] = array_filter($_POST['ips'], function ($ip) use ($mainIP) {
-    return $ip !== $mainIP;
-});
+// Keep every additional IPv4 (exclude the primary IP and blanks), then append
+// the IPv6 address. Stored newline-separated, as WHMCS expects.
+$extraIps = array_values(array_filter($_POST['ips'], function ($ip) use ($mainIP) {
+    return $ip !== $mainIP && $ip !== '';
+}));
+
+if (!empty($_POST['ipv6'])) {
+    $extraIps[] = $_POST['ipv6'];
+}
 
 Capsule::table('tblhosting')->where('id', $service->id)->update(array(
     'username' => $_POST['username'],
     'password' => encrypt($_POST['root']),
     'dedicatedip' => $mainIP,
-    'assignedips' => (!array_shift($_POST['ips']) ? $_POST['ipv6'] : implode('\n', $_POST['ips']) . '\n' . $_POST['ipv6']),
+    'assignedips' => implode("\n", $extraIps),
 ));
 
 echo '*ok*';
diff --git a/modules/servers/ArkhostVPSAG/template/clientarea_direct.tpl b/modules/servers/ArkhostVPSAG/template/clientarea_direct.tpl
index 6435e31..29b26bb 100644
--- a/modules/servers/ArkhostVPSAG/template/clientarea_direct.tpl
+++ b/modules/servers/ArkhostVPSAG/template/clientarea_direct.tpl
@@ -651,7 +651,7 @@ if (arkhostVNCHashDetected) {
 
 <script type="text/javascript">
     var productURL = '{$WEB_ROOT}/clientarea.php?action=productdetails&id={$serviceid}';
-    var serverInfoInitial = JSON.parse('{$serverInfo|json_encode}');
+    var serverInfoInitial = {$serverInfoJson nofilter};
     var lang = {
         moduleactionfailed: 'Action failed!',
         moduleactionsuccess: 'Action completed successfully',
@@ -907,6 +907,16 @@ if (arkhostVNCHashDetected) {
         return false;
     }
     
+    // Escape untrusted values before inserting them into innerHTML.
+    function escapeHtml(value) {
+        return String(value == null ? '' : value)
+            .replace(/&/g, '&amp;')
+            .replace(/</g, '&lt;')
+            .replace(/>/g, '&gt;')
+            .replace(/"/g, '&quot;')
+            .replace(/'/g, '&#39;');
+    }
+
     function updateBackupTable(data) {
         var tableBody = document.querySelector('#backupTable tbody');
         tableBody.innerHTML = '';
@@ -956,9 +966,9 @@ if (arkhostVNCHashDetected) {
                         statusBadge = '<span class="badge badge-danger">' + lang.backups.error + '</span>';
                     }
                     
-                    row.innerHTML = 
-                        '<td>' + backupDate + '</td>' +
-                        '<td>' + (backup.size || 'N/A') + '</td>' +
+                    row.innerHTML =
+                        '<td>' + escapeHtml(backupDate) + '</td>' +
+                        '<td>' + escapeHtml(backup.size || 'N/A') + '</td>' +
                         '<td>' + backupType + '</td>' +
                         '<td>' + statusBadge + '</td>' +
                         '<td style="white-space: nowrap;">' +
@@ -1075,12 +1085,12 @@ if (arkhostVNCHashDetected) {
             rulesList.forEach(function(rule) {
                 if (rule && rule.id) {
                     var row = document.createElement('tr');
-                    row.innerHTML = 
-                        '<td>' + (rule.action || 'N/A') + '</td>' +
-                        '<td>' + (rule.port || 'N/A') + '</td>' +
-                        '<td>' + (rule.protocol || 'N/A') + '</td>' +
-                        '<td>' + (rule.source || 'N/A') + '</td>' +
-                        '<td>' + (rule.note || 'N/A') + '</td>' +
+                    row.innerHTML =
+                        '<td>' + escapeHtml(rule.action || 'N/A') + '</td>' +
+                        '<td>' + escapeHtml(rule.port || 'N/A') + '</td>' +
+                        '<td>' + escapeHtml(rule.protocol || 'N/A') + '</td>' +
+                        '<td>' + escapeHtml(rule.source || 'N/A') + '</td>' +
+                        '<td>' + escapeHtml(rule.note || 'N/A') + '</td>' +
                         '<td>' +
                             '<a href="#" onclick="ArkHostVPS_API(\'Delete Firewall rule\', true, { rule_id: \'' + rule.id + '\' }); return false;" title="Delete">' +
                                 '<i class="fa fa-trash text-danger"></i>' +
@@ -1575,16 +1585,16 @@ if (arkhostVNCHashDetected) {
                                     <table class="table table-sm table-borderless mb-0">
                                         <tr>
                                             <td class="text-muted" width="40%">{$_LANG['Overview']['Hostname']}:</td>
-                                            <td><strong>{$serverInfo['hostname']|default:'N/A'}</strong></td>
+                                            <td><strong>{$serverInfo['hostname']|default:'N/A'|escape}</strong></td>
                                         </tr>
                                         <tr>
                                             <td class="text-muted">{$_LANG['IPv4']} Address:</td>
-                                            <td><code>{$serverInfo['ip']}</code></td>
+                                            <td><code>{$serverInfo['ip']|escape}</code></td>
                                         </tr>
                                         {if $serverInfo['ipv6']}
                                         <tr>
                                             <td class="text-muted">{$_LANG['IPv6']} Address:</td>
-                                            <td><code style="font-size: 11px;">{$serverInfo['ipv6']}</code></td>
+                                            <td><code style="font-size: 11px;">{$serverInfo['ipv6']|escape}</code></td>
                                         </tr>
                                         {/if}
                                         <tr>
@@ -1593,7 +1603,7 @@ if (arkhostVNCHashDetected) {
                                                 {if $serverInfo['operatingSystem']['image']}
                                                 <img src="{$serverInfo['operatingSystem']['image']}" width="20" height="20" class="mr-1" style="vertical-align: middle;">
                                                 {/if}
-                                                {$serverInfo['operatingSystem']['name']|default:'N/A'}
+                                                {$serverInfo['operatingSystem']['name']|default:'N/A'|escape}
                                             </td>
                                         </tr>
                                     </table>
@@ -1605,17 +1615,17 @@ if (arkhostVNCHashDetected) {
                                             <td>
                                                 <img src="{$serverInfo['statusImage']}" height="20" class="mr-1" style="vertical-align: middle;">
                                                 <span class="{if $serverInfo['status'] == 'running'}text-success{else}text-danger{/if}">
-                                                    <strong>{$serverInfo['statusDescription']}</strong>
+                                                    <strong>{$serverInfo['statusDescription']|escape}</strong>
                                                 </span>
                                             </td>
                                         </tr>
                                         <tr>
                                             <td class="text-muted">{$_LANG['Overview']['Uptime']}:</td>
-                                            <td><strong>{$serverInfo['uptime_text']|default:'N/A'}</strong></td>
+                                            <td><strong>{$serverInfo['uptime_text']|default:'N/A'|escape}</strong></td>
                                         </tr>
                                         <tr>
                                             <td class="text-muted">{$_LANG['Overview']['ServerType']}:</td>
-                                            <td><strong>{$serverInfo['server_type']|default:'Standard'}</strong></td>
+                                            <td><strong>{$serverInfo['server_type']|default:'Standard'|escape}</strong></td>
                                         </tr>
                                     </table>
                                 </div>
@@ -1993,7 +2003,7 @@ if (arkhostVNCHashDetected) {
                                         <div class="col-md-6">
                                             <div class="mb-3">
                                                 <label class="form-label">{$_LANG['Settings']['Hostname']['Title']}:</label>
-                                                <input class="form-control" id="hostnameRDNS" type="text" size="30" maxlength="128" value="{$serverInfo['hostname']}">
+                                                <input class="form-control" id="hostnameRDNS" type="text" size="30" maxlength="128" value="{$serverInfo['hostname']|escape}">
                                             </div>
                                         </div>
                                     </div>
@@ -2045,7 +2055,7 @@ if (arkhostVNCHashDetected) {
                                                 <label class="form-label">{$_LANG['Settings']['Password']['Title']}:</label>
                                                 
                                                 <div class="input-group mb-3">
-                                                    <input class="form-control" id="vpsPassword" type="password" size="30" maxlength="128" disabled value="{if $serverInfo['install_root'] != ''}{$serverInfo['install_root']}{else}Expired{/if}">
+                                                    <input class="form-control" id="vpsPassword" type="password" size="30" maxlength="128" disabled value="{if $serverInfo['install_root'] != ''}{$serverInfo['install_root']|escape}{else}Expired{/if}">
                                                     <button class="btn btn-outline-secondary" type="button" onclick="ArkHostVPS_ShowPassword();return false;">
                                                         <i class="fa fa-eye" id="showPasswordIcon" aria-hidden="true"></i>
                                                     </button>
@@ -2080,14 +2090,14 @@ if (arkhostVNCHashDetected) {
                                                         </div>
 
                                                         <div class="media-body float-left text-left p-2">
-                                                            <h6 class="distro_name media-heading">{$operatingSystemsGroup['name']}</h6>
+                                                            <h6 class="distro_name media-heading">{$operatingSystemsGroup['name']|escape}</h6>
                                                             <span id="{$group}-version" class="version small">{$_LANG['Settings']['Reinstall']['Version']}</span>
                                                         </div>
                                                     </button>
 
                                                     <div class="os_badge_list dropdown-menu w-100">
                                                         {foreach from=$operatingSystemsGroup['versions'] item=$operatingSystem}
-                                                        <a class="dropdown-item" href="#" data-os="{$operatingSystem['id']}" data-group="{$group}" onclick="ArkHostVPS_ChooseOS(this);return false;">{$operatingSystem['name']}</a>
+                                                        <a class="dropdown-item" href="#" data-os="{$operatingSystem['id']|escape}" data-group="{$group|escape}" onclick="ArkHostVPS_ChooseOS(this);return false;">{$operatingSystem['name']|escape}</a>
                                                         {/foreach}
                                                     </div>
                                                 </div>
diff --git a/modules/servers/ArkhostVPSAG/template/error.tpl b/modules/servers/ArkhostVPSAG/template/error.tpl
index 3c3b2c1..82eb3cc 100644
--- a/modules/servers/ArkhostVPSAG/template/error.tpl
+++ b/modules/servers/ArkhostVPSAG/template/error.tpl
@@ -23,7 +23,7 @@
 	<body class="text-center">
 		<div class="alert alert-warning" role="alert">
 			<div class="notice">
-				<img src="{$image}" class="mr-2">{$error}
+				<img src="{$image}" class="mr-2">{$error|escape}
 			</div>
 		</div>
 	</body>