Skip to content

fix(configSync): require HTTPS for sync host to prevent MITM RCE#11228

Merged
Eugeny merged 1 commit into
Eugeny:masterfrom
sebastiondev:fix/cwe295-configsync-service-config-cd16
May 7, 2026
Merged

fix(configSync): require HTTPS for sync host to prevent MITM RCE#11228
Eugeny merged 1 commit into
Eugeny:masterfrom
sebastiondev:fix/cwe295-configsync-service-config-cd16

Conversation

@sebastiondev
Copy link
Copy Markdown
Contributor

Summary

ConfigSyncService.request() in tabby-settings/src/services/configSync.service.ts issues sync requests to whatever host the user has configured, including plain http:// URLs. The response body is parsed as YAML and merged into the local Tabby configuration. Because Tabby profiles include fields like command and env that are later executed by the terminal, a network attacker able to tamper with the sync response can achieve arbitrary command execution on the victim's machine on the next sync.

This is a CWE-295 / CWE-319 issue (improper certificate / cleartext transport for security-sensitive data). Severity is high in the http:// configuration scenario because the impact is local code execution, and the precondition (on-path network position) is realistic on untrusted networks (rogue Wi‑Fi, ARP/DNS spoofing on a LAN, hostile ISP, etc.).

Data flow

  1. User sets configSync.host to http://... (the UI accepts arbitrary URLs).
  2. ConfigSyncService.request() concatenates the host with the API path and calls axios.request({...}).
  3. On getConfig() / sync, the response body is YAML‑parsed and merged into the in‑process config.store.
  4. Profile entries from the merged config are later spawned by the terminal layer — attacker‑controlled command ⇒ RCE.

There is no parallel HTTP entrypoint for this service; every API call funnels through request(), so guarding it is sufficient.

Fix

In request(), before performing the axios call, validate that the configured host begins with https:// (case‑insensitive). If not, log and throw. The check uses /^https:\/\//i so it correctly rejects http://, ftp://, scheme‑confusion (https-evil://), protocol‑relative URLs (//host), and leading‑whitespace tricks, while accepting any case of https://.

if (!/^https:\/\//i.test(host)) {
    const message = `Config sync host must use HTTPS (got: ${host})`
    this.logger.error(message)
    throw new Error(message)
}

Axios on Node validates TLS certificates by default, so requiring https:// provides meaningful integrity and authenticity guarantees against an on‑path attacker.

The diff is intentionally minimal (12 added lines, 1 changed) and confined to the single chokepoint.

What was tested

  • Regex behaviour against the relevant inputs:
    • Rejects: http://example.com, HTTP://example.com, ftp://example.com, https-evil://x, //example.com, https://example.com (leading whitespace), empty string.
    • Accepts: https://example.com, HTTPS://example.com, HtTpS://example.com.
  • Reviewed call sites of request() in configSync.service.ts (getConfigs, getConfig, setConfig, deleteConfig) — all flow through the guard.
  • Confirmed there is no alternate HTTP path that bypasses request() in this service.
  • Confirmed the YAML payload is merged into the live config and that profile command/env are subsequently executed by the terminal subsystem (the sink that makes this exploitable rather than merely an information disclosure).

Security analysis

The exploit chain is concrete: MITM the YAML response → inject a profile with a malicious command → user runs that profile (or, depending on how merged config is consumed, code path triggers automatically) → arbitrary code execution under the user's account. HTTPS with default certificate validation (which axios performs on Node) breaks the chain by preventing silent tampering and impersonation by an on‑path attacker.

The trade‑off is that users who deliberately configured an http:// sync endpoint (e.g., a LAN service without TLS) will now see an explicit error instead of a silent insecure sync. That seems clearly correct: the sync payload is executable‑equivalent, and there is no safe way to fetch it over cleartext.

Adversarial review

Before submitting, we tried to disprove this finding. The questions we considered were: (1) does axios or the framework already enforce HTTPS? — no, axios honours whatever scheme the URL specifies. (2) Does the YAML merge actually reach an executable sink, or is it inert configuration? — profile command/env fields are spawned by the terminal layer, so the merged data is executable‑equivalent. (3) Is the precondition (on‑path network position) already equivalent to code execution, making the fix pointless? — no, MITM on a public/Wi‑Fi/LAN network does not by itself yield RCE on the victim host; that step is precisely what the cleartext sync grants the attacker. The finding holds.

cc @lewiswigmore

@Eugeny Eugeny merged commit ed3fbff into Eugeny:master May 7, 2026
10 checks passed
@Eugeny
Copy link
Copy Markdown
Owner

Eugeny commented May 7, 2026

Thank you! @all-contributors please add @sebastiondev for code

@allcontributors
Copy link
Copy Markdown
Contributor

@Eugeny

We had trouble processing your request. Please try again later.

@sebastiondev
Copy link
Copy Markdown
Contributor Author

Appreciate the review. @lewiswigmore

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants