Skip to content

[SECURITY] - Fix four externally reachable security findings #175

Description

@davidindra

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 4c86806CryptographicOperations.FixedTimeEquals

Thanks in advance!

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions