Repository navigation
Triage: external vulnerability report against 0.1.2-alpha.6 (13 findings) #157
Description
Triage of an external report of 13 findings, written against 0.1.2-alpha.6 (russh 0.46, tauri 2.11.5, russh-sftp 3.0.0). Each finding was re-checked against current main (894ea12) and the dependency versions in Cargo.lock: russh 0.63.3, russh-cryptovec 0.62.0, russh-sftp 3.0.1, tauri 2.12.1.
Summary
| Key | Finding | Report | Verdict | Where |
|---|---|---|---|---|
| C1-A1-H1 | read_key_file reads any local path |
high | fixed on main | #107: the command is gone; pick_key_file opens the native dialog from Rust |
| C1-A1-H2 | read_key_file size check bypassed by procfs files |
low | fixed on main | #107 (no page-supplied path) + #120 (cap enforced on the read itself) |
| C1-A2-H1 | sftp_download_to_local writes any local path |
high | fixed on main | #107: the backend opens the save dialog itself |
| C1-A2-H2 | sftp_upload_from_local reads any local path |
medium | fixed on main | #107: single-use tokens from a native pick |
| C4-A2-H1 | server-chosen name steers the save dialog's path | high | fixed on main, hardened further | #107 + #120 (remote_file_name); bidi and device-name hardening in #155 |
| C3-A2-H2 | dialog:default lets the page fake host-key prompts |
high | fixed on main | #107 removed every dialog grant; #123 adds a test that keeps them out |
| C3-A1-H1 | curve25519 f length ≠ 32 panics under panic = "abort" |
high | fixed by upgrade | russh 0.63.3 checks the length (kex/curve25519.rs:115) |
| C3-A1-H2 | all-zero X25519 secret panics in the mpint encoder | high | fixed by upgrade | russh 0.63.3 rejects a zero secret and fixed encode_mpint |
| C4-A3-H1 | russh-sftp read_packet(stream, u32::MAX) allocation |
medium | fixed by upgrade | russh-sftp 3.0.1 caps packets at 256 KiB |
| C3-A2-H1 | held host-key change outlives the pinned key verifying again | high | valid (medium) | #137 → #138 |
| C4-A3-H2 | endless read_dir under the manager-wide SFTP mutex |
medium | valid (medium) | #139 → #140 (narrower lock, 30 s deadline, vault-reset drain restored) |
| C4-A1-H1 | unbounded terminal-output queue in the Tauri channel | high | partially valid (medium) | tauri 2.12.1 made the queue per-webview and clears it on close, but it is still unbounded while the webview lives. #158 → #159 (credit-based flow control, stacked on #140) |
| C1-A3-H1 | host and identity writes persist without confirmation | high | valid (low on main) | #122 (open) adds the native confirmation |
| C1-A2-H3 | state files have no integrity check | high | won't fix | see below |
Notes
Severity. Most renderer-side findings assume script running in the webview. No XSS path was found: there is no v-html / innerHTML / eval, the CSP has no unsafe-eval and no remote scripts, and xterm has no link addon. Script in the page can also already open a local shell through create_local_terminal + session_write (#30), which outranks every file-access finding. So the "high" ratings above are mostly overstated for current main. The dependency DoS findings require the user to connect to the hostile server (UI:R, not UI:N).
State file integrity (won't fix).
- Delivery channel already closed. The report's channel for overwriting the files (a renderer-chosen download path) is closed by Keep decrypted key material wiped, and store imported keys without their passphrase #107.
- Startup lockout already handled. Land the six security changes that PRs #100-#106 merged into each other instead of main, and answer their review #120 opens a recovery screen.
- Vault rollback already detected. Land the six security changes that PRs #100-#106 merged into each other instead of main, and answer their review #120 records a per-device epoch in the OS credential store.
- A MAC would not stop the remaining attacker. What is left is same-UID code that can rewrite the app data directory, and a MAC over
store.json/known_hosts.jsonwould not stop it. The key can't come from the vault, because both files are needed before unlock. A keychain-held key is readable by the same user, and that attacker can patch the binary anyway. - Recorded here as accepted residual risk.
Release. 0.1.2-alpha.6 is still the latest release, and it predates #107 and #120. Everything marked "fixed on main" reaches users only with the next release.
Found during the same pass (not in the report)
- One stalled SSH or SFTP peer blocks every other session and keeps vault reset from cutting it off #139 / Keep the session maps unlocked across network I/O #140: one stalled SSH peer also freezes every other SSH tab (the same lock pattern in
SessionManager). - A failed SFTP download truncates the file it was overwriting, and uploads use 8 KiB writes #141 / Leave the original file alone when a download fails, and widen upload writes #142: a failed download truncates the file it was overwriting; uploads are capped at 128 KiB in flight.
- An unsigned latest.json can roll every client back to an older signed release #143 / Bind update signatures to their version and take manifests only from our releases #144: an unsigned
latest.jsoncan roll clients back to an older signed release. - CI and release hardening: unenforced lockfile, release.sh key exposure, and settings that leave the signing key reachable #145 / Build against the committed Cargo.lock, bound CI jobs, and keep the updater key out of local release builds #146: CI and release hardening;
release.shexposed the updater key. - Server-opened channels are accepted, and the host startup command is never run #147 / Refuse channels the server opens back, and type the startup command into the shell #148: server-opened channels accepted; startup command never ran.
- Terminal input: pastes can escape bracketed paste, OSC 8 links open popups, keystrokes can reorder #149 / Harden terminal input against hostile pastes, links and reordering #150: bracketed-paste breakout, OSC 8 popups, keystroke reordering.
- Saving the vault leaves copies of every private key in freed memory #151 / Wipe every buffer a vault save serializes private keys into #152: vault save leaves key copies in freed memory.
- Closed local terminals leave zombie shells, and two Tauri plugins are registered for nothing #153 / End and reap a local shell when its tab closes, and drop the unused os and process plugins #154: zombie local shells; unused os/process plugins.
- The first connection to a host trusts any key without showing its fingerprint #156: first connection trusts any host key silently (proposal, needs a decision).
Activity
Sadykhzadeh commentedon Oct 17, 2026
@computerbox124, here is the merge order for the open security and hardening PRs.
I trial-merged all of them on top of
mainin this order, together with #122 and #123, and rancargo test --workspaceon the result. Everything passes except the expected ACL check between #123 and #159, described below.Order
security-audit; every other PR shows it red only because of the advisories this one fixes. Merge it first.fix/session-lock-scope(Keep the session maps unlocked across network I/O #140). After Keep the session maps unlocked across network I/O #140 merges, switch each base tomain(Edit → base); GitHub may also retarget them automatically when the branch is deleted. Merge in any order. CI only runs once the base ismain.CODEOWNERS(lists you and @Sadykhzadeh). Independent.Conflicts to expect
capabilities/default.json(os:default). Take Put the app's commands behind the capability ACL, trim plugin grants, keep the window on the app's pages #123's version of the file; End and reap a local shell when its tab closes, and drop the unused os and process plugins #154 removes the os/process plugins, so nothing else needs to change.session_output_ackcommand. Put the app's commands behind the capability ACL, trim plugin grants, keep the window on the app's pages #123'stests/app_command_acl.rswill fail until that command gets anallow-session-output-ackentry inpermissions/app-commands.tomland a grant incapabilities/default.json. Whichever of the two lands second needs those two lines. I'll push that fix-up to whichever PR is still open.Everything else merges cleanly in this order.
Before the next release
requireSignedVersion, so every release needstauri-cli ≥ 2.12with--app-version, or updated clients will refuse it.release.yml(Bind update signatures to their version and take manifests only from our releases #144) andscripts/release.sh(Build against the committed Cargo.lock, bound CI jobs, and keep the updater key out of local release builds #146) already do this, and the CI sign job checks it before uploading.0.1.2-alpha.6predates Keep decrypted key material wiped, and store imported keys without their passphrase #107/Land the six security changes that PRs #100-#106 merged into each other instead of main, and answer their review #120 and everything above, so a new release is worth cutting once these land.releaseenvironment holdingTAURI_SIGNING_*;v*;Sadykhzadeh commentedon Oct 17, 2026
@computerbox124, an update to the merge order above, now covering everything opened since. I trial-merged all open PRs onto
mainin this order, then checked every conflict pairwise to see which PR it actually collides with.Order
security-auditgreen. Consolidate the four open dependency bumps and clear the npm production audit #128 fixes the rustls/quinn-proto advisories. Suppress the updater's zip extraction advisories with a review date #186 suppresses three newzipadvisories thattauri-plugin-updaterpins us to: no upstream fix exists, and the vulnerable path is only reachable with a validly signed update. Any order between the two.core/src/vault.rs.core/src/connection.rs.window.confirmnever blocks in the packaged app (Confirmations never block: the dialog plugin turns window.confirm into a Promise #168).mainafter Ask for destructive confirmations in the page, since window.confirm never blocks in the app #172 merges.confirmActionafter Ask for destructive confirmations in the page, since window.confirm never blocks in the app #172.App.vue.TerminalPane.vue.capabilities/default.jsoncase already noted above.Conflicts
Every conflict above is in a file both PRs touch for unrelated reasons; none of them changes behaviour. Merge in the order above and leave the rest to me. After each merge I'll rebase whichever open PR then conflicts, rerun its tests, and push, so the next one is always mergeable.
Bot review threads
Every Qodo thread on these PRs has been answered: fixed with a test, or replied to with the reasoning. The one exception is #167's Workspace Open thread, which waits for #172.