aleleba/wordcraft
1
0
Fork 0
mirror of https://github.com/storytold/wordcraft.git synced 2026-10-27 11:57:36 -06:00
Code Issues Packages Projects Releases Wiki Activity

#3699: MCP bridge sends the control key only to 127.0.0.1, where the app listens

Browse Source 
Audit finding 9 (CWE-522). The bridge sent the reusable control key to any
loopback peer (127.0.0.0/8, ::1) and tried every address `localhost`
resolved to, while the app binds only 127.0.0.1. Another local user could
listen on ::1 or 127.0.0.2 and receive the key. Now `localhost` connects to
127.0.0.1 only, and the key goes only to a 127.0.0.1 peer.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Martin Ball
2026-10-23 20:19:49 +01:00
co-authored by Claude Opus 5.5
parent 9501975262
commit 08da5c5433
2 changed files with 63 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/mcp/src/backend.rs
+60 -26
View File
@@ -1,7 +1,7 @@
//! Where MCP tool calls end up: a control-channel method call.
use std::io::{BufRead, BufReader, Write};
use std::net::{IpAddr, SocketAddr, TcpStream, ToSocketAddrs};
use std::net::{IpAddr, Ipv4Addr, SocketAddr, TcpStream, ToSocketAddrs};
use std::path::Path;
use std::time::Duration;
@@ -26,7 +26,7 @@ pub struct Remote {
addr: String,
conn: Option<(BufReader<TcpStream>, TcpStream)>,
next_id: u64,
/// The connection leads to a loopback address: requests carry the control key.
/// The connection leads to the app's 127.0.0.1: requests carry the control key.
send_key: bool,
/// The control key, looked up once per connection (the app picks a new one at every start).
key: Option<String>,
@@ -55,25 +55,46 @@ fn find_key(addr: &str) -> Result<String, String> {
lookup_key(std::env::var("WORDCRAFT_CONTROL_KEY").ok(), wordcraft_control_key::settings_dir().as_deref(), port)
}
/// Whether `addr` (`HOST:PORT`, as given to `--connect`) names a loopback address: 127.0.0.0/8,
/// ::1 or `localhost`. Other host names are not resolved: they never count.
fn is_loopback_addr(addr: &str) -> bool {
let loopback = |ip: IpAddr| ip.to_canonical().is_loopback();
if let Ok(sa) = addr.parse::<SocketAddr>() {
return loopback(sa.ip());
}
if let Ok(ip) = addr.parse::<IpAddr>() {
return loopback(ip);
}
/// The only address the app's control server binds (`apps/wordcraft/src/control_server.rs`).
const APP_IP: IpAddr = IpAddr::V4(Ipv4Addr::LOCALHOST);
/// The host part of `addr` (`HOST:PORT`, `[V6]:PORT`), without brackets.
fn host_of(addr: &str) -> &str {
let host = addr.rsplit_once(':').map_or(addr, |(h, _)| h);
let host = host.strip_prefix('[').and_then(|h| h.strip_suffix(']')).unwrap_or(host);
host.parse::<IpAddr>().map_or_else(|_| host.eq_ignore_ascii_case("localhost"), loopback)
host.strip_prefix('[').and_then(|h| h.strip_suffix(']')).unwrap_or(host)
}
/// The key goes only to a loopback `--connect` address, and only when the connection really
/// leads to one (`peer`, so a `localhost` that resolves elsewhere gets no key either).
/// Whether `addr` (`HOST:PORT`, as given to `--connect`) names the app's own address:
/// 127.0.0.1 or `localhost`. Other loopback addresses (127.0.0.2, ::1) don't count: the app never
/// listens there, so whatever does is not the app. Other host names are not resolved.
fn is_app_addr(addr: &str) -> bool {
let app = |ip: IpAddr| ip.to_canonical() == APP_IP;
if let Ok(sa) = addr.parse::<SocketAddr>() {
return app(sa.ip());
}
if let Ok(ip) = addr.parse::<IpAddr>() {
return app(ip);
}
let host = host_of(addr);
host.parse::<IpAddr>().map_or_else(|_| host.eq_ignore_ascii_case("localhost"), app)
}
/// The key goes only to the app's `--connect` address, and only when the connection really
/// leads to 127.0.0.1 (`peer`): a listener on another loopback address (::1, 127.0.0.2), which
/// any local user can open, never sees it.
fn may_send_key(addr: &str, peer: Option<SocketAddr>) -> bool {
is_loopback_addr(addr) && peer.is_some_and(|p| p.ip().to_canonical().is_loopback())
is_app_addr(addr) && peer.is_some_and(|p| p.ip().to_canonical() == APP_IP)
}
/// Where to connect for `addr`: `localhost` means the app's 127.0.0.1 (not whatever the resolver
/// lists first, which may be ::1); anything else as resolved.
fn targets(addr: &str) -> std::io::Result<Vec<SocketAddr>> {
if host_of(addr).eq_ignore_ascii_case("localhost")
&& let Some(port) = addr.rsplit_once(':').and_then(|(_, p)| p.parse::<u16>().ok())
{
return Ok(vec![SocketAddr::new(APP_IP, port)]);
}
Ok(addr.to_socket_addrs()?.collect())
}
impl Remote {
@@ -95,11 +116,12 @@ impl Remote {
&self.addr
}
/// Why requests go without the control key (the address is not loopback), if they do.
/// Why requests go without the control key (the address is not the app's 127.0.0.1), if they
/// do.
pub fn key_warning(&self) -> Option<String> {
(!self.send_key).then(|| {
format!(
"{} is not a loopback address, so the control key is not sent (it only goes to 127.0.0.0/8, ::1 and localhost) and the app refuses requests; reach the app through 127.0.0.1, for example with an SSH tunnel",
"{} is not the app's address, so the control key is not sent (it only goes to 127.0.0.1 and localhost, where the app listens) and the app refuses requests; reach the app through 127.0.0.1, for example with an SSH tunnel",
self.addr
)
})
@@ -109,7 +131,7 @@ impl Remote {
self.conn = None;
self.key = None;
let mut last = std::io::Error::new(std::io::ErrorKind::NotFound, format!("cannot resolve {}", self.addr));
for sa in self.addr.to_socket_addrs()? {
for sa in targets(&self.addr)? {
match TcpStream::connect_timeout(&sa, Duration::from_millis(800)) {
Ok(s) => {
s.set_nodelay(true).ok();
@@ -256,15 +278,12 @@ mod tests {
}
#[test]
fn key_only_for_loopback_addresses() {
fn key_only_for_the_apps_address() {
let peer = |s: &str| s.parse::<SocketAddr>().ok();
for (addr, at) in [
("127.0.0.1:7981", "127.0.0.1:7981"),
("127.0.0.2:7981", "127.0.0.2:7981"),
("[::1]:7981", "[::1]:7981"),
("[::ffff:127.0.0.1]:7981", "[::ffff:127.0.0.1]:7981"),
("localhost:7981", "127.0.0.1:7981"),
("localhost:7981", "[::1]:7981"),
("LocalHost:7981", "127.0.0.1:7981"),
] {
assert!(may_send_key(addr, peer(at)), "{addr} at {at}");
@@ -274,6 +293,11 @@ mod tests {
("10.0.0.7:7981", "10.0.0.7:7981"),
("[fe80::1]:7981", "[fe80::1]:7981"),
("0.0.0.0:7981", "127.0.0.1:7981"),
// Loopback addresses the app never binds: another local user's listener (#3699).
("127.0.0.2:7981", "127.0.0.2:7981"),
("[::1]:7981", "[::1]:7981"),
("localhost:7981", "[::1]:7981"),
("127.0.0.1:7981", "127.0.0.2:7981"),
// Host names other than localhost: even when they lead to this machine.
("workstation:7981", "127.0.1.1:7981"),
("example.org:7981", "93.184.215.14:7981"),
@@ -287,6 +311,16 @@ mod tests {
assert!(!may_send_key("127.0.0.1:7981", None));
}
#[test]
fn localhost_connects_to_the_apps_address_only() {
// Not the resolver's list, which may put ::1 (where anyone can listen) first.
let at: SocketAddr = "127.0.0.1:7981".parse().unwrap();
assert_eq!(targets("localhost:7981").unwrap(), vec![at]);
assert_eq!(targets("LOCALHOST:7981").unwrap(), vec![at]);
assert_eq!(targets("127.0.0.1:7981").unwrap(), vec![at]);
assert_eq!(targets("[::1]:7981").unwrap(), vec!["[::1]:7981".parse::<SocketAddr>().unwrap()]);
}
/// A stand-in for the app on 127.0.0.1: every request line goes to the returned channel and
/// gets `answer(request)`: a reply and whether to close the connection after it, or `None`
/// to close it without a reply (the app quit).
@@ -377,9 +411,9 @@ mod tests {
let finder: KeyFinder = Box::new(|_| Err("the key must not be read for another host".to_string()));
let mut r = Remote::connect_with(&format!("127.0.0.1:{port}"), finder).unwrap();
r.send_key = false;
assert!(r.key_warning().is_some_and(|w| w.contains("not a loopback address")));
assert!(r.key_warning().is_some_and(|w| w.contains("not the app's address")));
let e = r.call("document.inspect", json!({})).unwrap_err();
assert!(e.contains("not a loopback address") && e.contains("not sent"), "{e}");
assert!(e.contains("not the app's address") && e.contains("not sent"), "{e}");
assert!(seen.recv_timeout(WAIT).unwrap().get("key").is_none());
}
}
docs/control-protocol.md
+3 -1
View File
@@ -43,7 +43,9 @@ cat ~/.config/wordcraft/control-key.local-7981 # Linux, outside Flatpak, port
`wordcraft-cli mcp --connect` takes the key from `WORDCRAFT_CONTROL_KEY`, or else from the key
file for the port it connects to, and reads it again on every new connection (the app may have
restarted). It sends the key only to loopback addresses (`127.0.0.0/8`, `::1`, `localhost`). For
restarted). It sends the key only to the app's own address, `127.0.0.1` (`localhost` connects
there too, never to `::1`): another loopback address such as `::1` or `127.0.0.2` is not where the
app listens, so whoever listens there gets no key (#3699). For
any other host it sends no key, says so, and the app refuses its requests: to drive an app on
another machine, forward its port to `127.0.0.1` (for example `ssh -L 7981:127.0.0.1:7981 host`)
and set `WORDCRAFT_CONTROL_KEY` to that app's key.