Skip to content

fix: harden ssh local tunnel binding against TOCTOU races#215

Merged
datlechin merged 1 commit intoTableProApp:mainfrom
eliottwantz:fix/toctou-attack
Mar 9, 2026
Merged

fix: harden ssh local tunnel binding against TOCTOU races#215
datlechin merged 1 commit intoTableProApp:mainfrom
eliottwantz:fix/toctou-attack

Conversation

@eliottwantz
Copy link
Copy Markdown
Contributor

@eliottwantz eliottwantz commented Mar 9, 2026

Closes #216

Summary

Harden SSH local port forwarding against a TOCTOU race during tunnel setup.

Previously, TablePro scanned for a free local port, released it, and only then launched ssh -L .... Another local process could bind that port in the gap between the availability check and the actual ssh bind.

This change removes that race by letting ssh perform the bind directly and retrying only when ssh reports a local forwarding bind failure.

What changed

  • Replaced the pre-bind findAvailablePort() flow with launch-and-retry logic over candidate ports
  • Added -o ExitOnForwardFailure=yes so local forwarding bind failures fail fast
  • Bound the local forward explicitly to 127.0.0.1
  • Tightened readiness checks so the forwarded port must:
    • accept connections, and
    • be owned by the launched process tree
  • Added focused unit tests for:
    • retryable local bind failures
    • process-tree ownership detection

Why

The old flow had a classic TOCTOU window:

  1. TablePro checked whether a local port was free
  2. The check released the port
  3. ssh later attempted to bind the same port

A competing local process could win that race and either:

  • cause ssh to fail binding, or
  • be mistaken for the expected tunnel if readiness only checked whether something was listening

This PR makes tunnel setup fail closed and retries only on the specific local bind race case.

Testing

  • Added unit coverage for bind-failure classification
  • Added unit coverage for process-tree ownership checks

Stop pre-scanning for an open local port before launching ssh.
Instead, let ssh bind the forwarded port directly with
`ExitOnForwardFailure=yes` and retry only when stderr shows a local
bind failure.

Also tighten tunnel readiness checks so the forwarded port must be
reachable and owned by the launched process tree, preventing false
positives if another local process grabs the port first.

Add focused tests for retryable bind failures and process-tree
ownership checks.
@chatgpt-codex-connector
Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 9, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@eliottwantz
Copy link
Copy Markdown
Contributor Author

I have read the CLA Document and I hereby sign the CLA.

github-actions bot added a commit that referenced this pull request Mar 9, 2026
@datlechin datlechin merged commit a96443a into TableProApp:main Mar 9, 2026
2 of 3 checks passed
@eliottwantz eliottwantz deleted the fix/toctou-attack branch March 9, 2026 03:53
@eliottwantz eliottwantz restored the fix/toctou-attack branch March 9, 2026 03:56
@eliottwantz eliottwantz deleted the fix/toctou-attack branch March 9, 2026 03:58
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.

Bug: SSH Tunnel TOCTOU race attack vulnerability

2 participants