diff --git a/backend/app/Chat/Node.php b/backend/app/Chat/Node.php index 5345a472b..5185602bc 100755 --- a/backend/app/Chat/Node.php +++ b/backend/app/Chat/Node.php @@ -435,7 +435,7 @@ public static function searchNodes( $params['exclude_node_id'] = $excludeNodeId; } - $sql .= ' ORDER BY n.' . $sortBy . ' ' . $sortOrder; + $sql .= ' ORDER BY n.' . self::sanitizeSortColumn($sortBy) . ' ' . (strtoupper($sortOrder) === 'DESC' ? 'DESC' : 'ASC'); $sql .= ' LIMIT :limit OFFSET :offset'; $stmt = $pdo->prepare($sql); @@ -625,6 +625,19 @@ public static function getColumns(): array return $stmt->fetchAll(\PDO::FETCH_ASSOC); } + /** + * Allowlist for ORDER BY column names in searchNodes(). $sortBy is not + * currently passed through from any route (index() uses the defaults), + * but validating it here closes the SQL injection vector defensively in + * case that changes later, consistent with Spell::getColumns()/Mount.php. + */ + private static function sanitizeSortColumn(string $sortBy): string + { + $allowed = ['id', 'uuid', 'name', 'fqdn', 'location_id', 'created_at', 'updated_at']; + + return in_array($sortBy, $allowed, true) ? $sortBy : 'name'; + } + /** * Generate a cryptographically secure UUID for nodes. */ diff --git a/backend/app/Controllers/Admin/NodesController.php b/backend/app/Controllers/Admin/NodesController.php index 2e1945940..b2b01be80 100755 --- a/backend/app/Controllers/Admin/NodesController.php +++ b/backend/app/Controllers/Admin/NodesController.php @@ -257,6 +257,13 @@ public function index(Request $request): Response $nodes = Node::searchNodes(page: $page, limit: $limit, search: $search, locationId: $locationId, excludeNodeId: $excludeNodeId); $total = Node::getNodesCount(search: $search, locationId: $locationId, excludeNodeId: $excludeNodeId); + // See enrichNode() for why the daemon token must not appear in list + // responses (it grants full API access to the node). + foreach ($nodes as &$node) { + unset($node['daemon_token'], $node['daemon_token_id']); + } + unset($node); + $totalPages = ceil($total / $limit); $from = ($page - 1) * $limit + 1; $to = min($from + $limit - 1, $total); @@ -2227,6 +2234,15 @@ private function enrichNode(array $node): array $node['daemon_type'] = $caps->getType(); $node['capabilities'] = $caps->toArray(); + // The daemon token grants full API access to the node. It must not + // be returned from general list/detail responses - only the + // dedicated /setup-command endpoint (which requires the same admin + // permission but is an explicit, auditable "reveal" action) should + // expose it. Leaving it in every index()/show() response meant it + // could end up in browser history, proxy/access logs, or dev tools + // for any admin who merely views the nodes list. + unset($node['daemon_token'], $node['daemon_token_id']); + return $node; } diff --git a/backend/app/Controllers/Admin/UsersController.php b/backend/app/Controllers/Admin/UsersController.php index d1c33135d..721dfc159 100755 --- a/backend/app/Controllers/Admin/UsersController.php +++ b/backend/app/Controllers/Admin/UsersController.php @@ -834,6 +834,28 @@ public function update(Request $request, string $uuid): Response $data['reason'] ); + // Privilege-escalation guard: this endpoint only requires + // ADMIN_USERS_EDIT, but several columns are security-sensitive + // (role, 2FA state, session token). An admin with only "edit users" + // permission (not full ADMIN_ROOT) must not be able to grant + // themselves or another account a higher role, disable/hijack 2FA, + // or forge a session by setting remember_token directly. Restrict + // those fields to ADMIN_ROOT regardless of what the DB-column-based + // denylist above allows through. + $rootOnlyFields = ['role_id', 'two_fa_enabled', 'two_fa_key', 'two_fa_blocked', 'remember_token', 'external_id', 'deleted']; + $staffUuid = $request->attributes->get('user')['uuid'] ?? null; + $isRootAdmin = $staffUuid !== null && \App\Helpers\PermissionHelper::hasPermission($staffUuid, \App\Permissions::ADMIN_ROOT); + if (!$isRootAdmin) { + $attemptedRootFields = array_intersect($rootOnlyFields, array_keys($data)); + if (!empty($attemptedRootFields)) { + return ApiResponse::error( + 'You do not have permission to modify: ' . implode(', ', $attemptedRootFields), + 'INSUFFICIENT_PERMISSIONS_FOR_FIELD', + 403 + ); + } + } + if ($app->isDemoMode()) { if ($user['id'] === 1) { return ApiResponse::error('Unmanaged actions are not permitted in demo mode', 'UNMANAGED_ACTIONS_NOT_PERMITTED', 400);