Greetings,
first of all, thank you for your amazing work on this project!
However, an external-perimeter security review of the server found four issues reachable from the internet without a foothold or an insider: two rated High, two Medium.
1. Emailed account links are built from the request Host header — CVSS 8.1
ForgotPassword.razor derives the reset link from NavigationManager.ToAbsoluteUri, which on a server-rendered page takes its base from the incoming request, and appsettings.json ships "AllowedHosts": "*", so a forged value is not rejected. Submitting the forgot-password form with a victim's email and an attacker-chosen Host produces a genuine email from the real server containing a valid reset token that points at the attacker's origin. The email-confirmation and email-change links are built the same way.
Preconditions: SMTP configured, the victim's address confirmed, no reverse proxy pinning the upstream Host, and the victim clicks.
2. A device with no stored public key can be taken over on the agent hub — CVSS 8.1
AgentHub carries no [Authorize], so hub methods are reachable anonymously. The deprecated unsigned UpdateDevice accepts any device whose stored PublicKey is empty, and UpdateDeviceSigned verifies the signature against the key the caller supplied in the same message when there is nothing stored to check against. Either path rewrites ConnectionId — the field that routes viewer commands — and adopts the caller's key permanently, so the takeover survives.
Preconditions: the device GUID and tenant GUID; with self-bootstrap on a single-tenant server, the tenant GUID is not needed. To measure exposure: SELECT count(*) FROM "Devices" WHERE "PublicKey" IS NULL OR "PublicKey" = '';
3. The self-bootstrap single-tenant restriction is bypassable — CVSS 6.5
The tenants.Count > 1 check runs only inside if (... && agentDto.TenantId == Guid.Empty). A caller who names an existing tenant GUID explicitly skips it, so "self-bootstrap is only allowed on single-tenant servers" is not enforced against a caller who declines to opt in.
Applies only when AllowAgentsToSelfBootstrap is enabled; it defaults to false.
4. Relay access tokens are compared with == — CVSS 6.8
SessionSignaler.ValidateToken uses an ordinary string comparison, which short-circuits on the first differing byte. The responder half of a relay session is unauthenticated (RequireAuthenticationForResponder is never set), so this token is the only check on that side.
Proposed fixes
A branch with one commit per issue is available at security-fixes (PR #174):
|
Commit |
| 1 |
c231966 — adds AppOptions.PublicBaseUrl and an IPublicUrlProvider used by all five emailed-link sites |
| 2 |
4fed504 — removes the unsigned UpdateDevice; restricts caller-supplied keys to bootstrapping a device the server has never seen |
| 3 |
02ac81b — runs the single-tenant check whenever self-bootstrap is the authority, taking the tenant from the server |
| 4 |
4c86806 — CryptographicOperations.FixedTimeEquals |
Thanks in advance!
Greetings,
first of all, thank you for your amazing work on this project!
However, an external-perimeter security review of the server found four issues reachable from the internet without a foothold or an insider: two rated High, two Medium.
1. Emailed account links are built from the request
Hostheader — CVSS 8.1ForgotPassword.razorderives the reset link fromNavigationManager.ToAbsoluteUri, which on a server-rendered page takes its base from the incoming request, andappsettings.jsonships"AllowedHosts": "*", so a forged value is not rejected. Submitting the forgot-password form with a victim's email and an attacker-chosenHostproduces a genuine email from the real server containing a valid reset token that points at the attacker's origin. The email-confirmation and email-change links are built the same way.Preconditions: SMTP configured, the victim's address confirmed, no reverse proxy pinning the upstream
Host, and the victim clicks.2. A device with no stored public key can be taken over on the agent hub — CVSS 8.1
AgentHubcarries no[Authorize], so hub methods are reachable anonymously. The deprecated unsignedUpdateDeviceaccepts any device whose storedPublicKeyis empty, andUpdateDeviceSignedverifies the signature against the key the caller supplied in the same message when there is nothing stored to check against. Either path rewritesConnectionId— the field that routes viewer commands — and adopts the caller's key permanently, so the takeover survives.Preconditions: the device GUID and tenant GUID; with self-bootstrap on a single-tenant server, the tenant GUID is not needed. To measure exposure:
SELECT count(*) FROM "Devices" WHERE "PublicKey" IS NULL OR "PublicKey" = '';3. The self-bootstrap single-tenant restriction is bypassable — CVSS 6.5
The
tenants.Count > 1check runs only insideif (... && agentDto.TenantId == Guid.Empty). A caller who names an existing tenant GUID explicitly skips it, so "self-bootstrap is only allowed on single-tenant servers" is not enforced against a caller who declines to opt in.Applies only when
AllowAgentsToSelfBootstrapis enabled; it defaults tofalse.4. Relay access tokens are compared with
==— CVSS 6.8SessionSignaler.ValidateTokenuses an ordinary string comparison, which short-circuits on the first differing byte. The responder half of a relay session is unauthenticated (RequireAuthenticationForResponderis never set), so this token is the only check on that side.Proposed fixes
A branch with one commit per issue is available at
security-fixes(PR #174):c231966— addsAppOptions.PublicBaseUrland anIPublicUrlProviderused by all five emailed-link sites4fed504— removes the unsignedUpdateDevice; restricts caller-supplied keys to bootstrapping a device the server has never seen02ac81b— runs the single-tenant check whenever self-bootstrap is the authority, taking the tenant from the server4c86806—CryptographicOperations.FixedTimeEqualsThanks in advance!