diff --git a/README.md b/README.md index 1b1d7f4..0824865 100644 --- a/README.md +++ b/README.md @@ -200,6 +200,35 @@ wormhole/ └── .goreleaser.yml ``` +## Security + +Wormhole includes hardening measures to protect users who inadvertently expose sensitive local files or infrastructure details through the tunnel. + +### Sensitive path blocking (CWE-441) + +By default, wormhole blocks requests to dotfiles and `node_modules` before they reach your local server: + +- `/.env`, `/.git`, `/.aws`, `/.ssh`, `/.docker`, and any other root-level dotfile — return **403 Forbidden** +- `/node_modules/` anywhere in the path — return **403 Forbidden** + +This prevents credentials and source control history from being served to the internet even if your local server would normally serve them. + +To disable (for users who genuinely need to serve these paths): + +```bash +WORMHOLE_NO_PATH_FILTER=1 wormhole http 3000 +``` + +### Inspector CORS hardening (CWE-942) + +The traffic inspector (`localhost:4040`) no longer sets `Access-Control-Allow-Origin: *`. CORS headers are only returned when the request's `Origin` matches the inspector's own address. This prevents malicious websites visited in the same browser session from reading tunnel traffic via cross-origin requests. + +The WebSocket upgrader's `CheckOrigin` is also locked down to the same policy. + +### Error message sanitization (CWE-200) + +When the local server is unreachable, wormhole returns a generic message (`Tunnel connected, but the local service is not responding.`) to the remote caller instead of the raw Go error string. Internal network topology details (host names, port numbers, error codes) are logged locally only and never sent through the tunnel. + ## Development ```bash diff --git a/internal/client/local.go b/internal/client/local.go index 61e5968..50c0c6b 100644 --- a/internal/client/local.go +++ b/internal/client/local.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "net/http" + "os" "strings" "time" @@ -13,9 +14,59 @@ import ( const localTimeout = 30 * time.Second +// pathFilterEnabled returns true unless WORMHOLE_NO_PATH_FILTER=1 is set. +// When filtering is enabled, requests to dotfiles and node_modules are +// blocked with 403 before reaching the local server (CWE-441). +func pathFilterEnabled() bool { + return os.Getenv("WORMHOLE_NO_PATH_FILTER") != "1" +} + +// isSensitivePath returns true when the given path should be blocked. +// Blocked patterns: +// - Any path segment starting with "." at the root (/.env, /.git, /.aws, ...) +// - Any path containing /node_modules/ +func isSensitivePath(path string) bool { + // Normalise to ensure leading slash. + if !strings.HasPrefix(path, "/") { + path = "/" + path + } + // Block root-level dotfiles: path starts with "/.". + if strings.HasPrefix(path, "/.") { + return true + } + // Block node_modules anywhere in the path. + if strings.Contains(path, "/node_modules/") || path == "/node_modules" { + return true + } + return false +} + +// blockedResponse returns a 403 HTTP response message for the given request ID. +func blockedResponse(id string) *transport.HTTPResponseMessage { + return &transport.HTTPResponseMessage{ + Type: transport.TypeHTTPResponse, + ID: id, + Status: http.StatusForbidden, + Headers: map[string]string{"Content-Type": "text/plain"}, + Body: base64.StdEncoding.EncodeToString([]byte("Forbidden")), + } +} + // ForwardToLocal takes an HTTP request message from the relay, forwards it // to the local server, and returns an HTTP response message. +// +// Security properties: +// - Sensitive paths (dotfiles, node_modules) are blocked with 403 by +// default (CWE-441). Set WORMHOLE_NO_PATH_FILTER=1 to disable. +// - When the local server is unreachable, a generic error message is +// returned to the tunnel; the raw Go error is written to stderr only +// (CWE-200). func ForwardToLocal(localAddr string, req *transport.HTTPRequestMessage) (*transport.HTTPResponseMessage, error) { + // --- Fix 2: Path filtering (CWE-441) --- + if pathFilterEnabled() && isSensitivePath(req.Path) { + return blockedResponse(req.ID), nil + } + // Build the local URL url := fmt.Sprintf("http://%s%s", localAddr, req.Path) @@ -52,12 +103,17 @@ func ForwardToLocal(localAddr string, req *transport.HTTPRequestMessage) (*trans client := &http.Client{Timeout: localTimeout} resp, err := client.Do(httpReq) if err != nil { + // --- Fix 3: Error sanitisation (CWE-200) --- + // Log the real error locally; send only a generic message through + // the tunnel so internal network topology is not exposed to remote + // visitors. + fmt.Fprintf(os.Stderr, "wormhole: local forward error: %v\n", err) return &transport.HTTPResponseMessage{ Type: transport.TypeHTTPResponse, ID: req.ID, Status: 502, Headers: map[string]string{"Content-Type": "text/plain"}, - Body: base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("Failed to reach local server: %s", err))), + Body: base64.StdEncoding.EncodeToString([]byte("Tunnel connected, but the local service is not responding.")), }, nil } defer resp.Body.Close() diff --git a/internal/client/security_test.go b/internal/client/security_test.go new file mode 100644 index 0000000..6231bd7 --- /dev/null +++ b/internal/client/security_test.go @@ -0,0 +1,110 @@ +package client + +import ( + "encoding/base64" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/wormhole-dev/wormhole/internal/transport" +) + +// makeSecReq is a convenience helper for security tests. +func makeSecReq(id, path string) *transport.HTTPRequestMessage { + return &transport.HTTPRequestMessage{ + Type: transport.TypeHTTPRequest, + ID: id, + Method: "GET", + Path: path, + Headers: map[string]string{}, + Body: nil, + } +} + +// TestPathFilter_DotfileAtRoot verifies that /.env is blocked (CWE-441). +func TestPathFilter_DotfileAtRoot(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Error("request should have been blocked before reaching local server") + w.WriteHeader(200) + })) + defer srv.Close() + + resp, err := ForwardToLocal(srv.Listener.Addr().String(), makeSecReq("pf1", "/.env")) + require.NoError(t, err) + assert.Equal(t, http.StatusForbidden, resp.Status, "/.env must be blocked") +} + +// TestPathFilter_DotGit verifies that /.git/config is blocked (CWE-441). +func TestPathFilter_DotGit(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Error("request should have been blocked before reaching local server") + w.WriteHeader(200) + })) + defer srv.Close() + + resp, err := ForwardToLocal(srv.Listener.Addr().String(), makeSecReq("pf2", "/.git/config")) + require.NoError(t, err) + assert.Equal(t, http.StatusForbidden, resp.Status, "/.git/config must be blocked") +} + +// TestPathFilter_NodeModules verifies that /node_modules/package.json is blocked (CWE-441). +func TestPathFilter_NodeModules(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Error("request should have been blocked before reaching local server") + w.WriteHeader(200) + })) + defer srv.Close() + + resp, err := ForwardToLocal(srv.Listener.Addr().String(), makeSecReq("pf3", "/node_modules/package.json")) + require.NoError(t, err) + assert.Equal(t, http.StatusForbidden, resp.Status, "/node_modules/package.json must be blocked") +} + +// TestPathFilter_NormalPath verifies that a normal API path passes through. +func TestPathFilter_NormalPath(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + resp, err := ForwardToLocal(srv.Listener.Addr().String(), makeSecReq("pf4", "/api/hello")) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, resp.Status, "/api/hello must pass through") +} + +// TestPathFilter_SubdirDotfile verifies that /public/.hidden is NOT blocked — +// only root-level dotfiles (paths starting with "/.") are blocked. +func TestPathFilter_SubdirDotfile(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + resp, err := ForwardToLocal(srv.Listener.Addr().String(), makeSecReq("pf5", "/public/.hidden")) + require.NoError(t, err) + // /public/.hidden does NOT start with "/." so it should pass through. + assert.Equal(t, http.StatusOK, resp.Status, "/public/.hidden should pass through (not a root dotfile)") +} + +// TestErrorSanitization verifies that 502 error body is generic (CWE-200). +func TestErrorSanitization(t *testing.T) { + req := makeSecReq("san1", "/api/data") + + resp, err := ForwardToLocal("127.0.0.1:1", req) + require.NoError(t, err) + assert.Equal(t, 502, resp.Status) + + body, decErr := base64.StdEncoding.DecodeString(resp.Body) + require.NoError(t, decErr) + bodyStr := string(body) + + // Generic message must be present. + assert.Contains(t, bodyStr, "local service is not responding") + // Internal network details must NOT be in the response body. + assert.NotContains(t, bodyStr, "localhost") + assert.NotContains(t, bodyStr, "127.0.0.1") + assert.NotContains(t, bodyStr, "connect:") + assert.NotContains(t, bodyStr, "dial ") +} diff --git a/internal/inspect/server.go b/internal/inspect/server.go index 4770a29..bf40e4e 100644 --- a/internal/inspect/server.go +++ b/internal/inspect/server.go @@ -23,14 +23,40 @@ type Server struct { // NewServer creates an inspector server. func NewServer(recorder *Recorder, localAddr string, logger zerolog.Logger) *Server { - return &Server{ + s := &Server{ recorder: recorder, localAddr: localAddr, logger: logger, - upgrader: websocket.Upgrader{ - CheckOrigin: func(r *http.Request) bool { return true }, + } + s.upgrader = websocket.Upgrader{ + // CheckOrigin validates that the WebSocket upgrade request originates + // from the inspector itself (same host). This prevents cross-origin + // WebSocket hijacking (CWE-942). + CheckOrigin: func(r *http.Request) bool { + return s.isAllowedOrigin(r) }, } + return s +} + +// isAllowedOrigin returns true when the request's Origin header is either +// absent (e.g., curl / same-origin browser fetch) or matches the inspector's +// own address. This is the gating predicate for both CORS and WebSocket +// origin validation (CWE-942). +func (s *Server) isAllowedOrigin(r *http.Request) bool { + origin := r.Header.Get("Origin") + if origin == "" { + // No Origin header — direct browser navigation or non-browser client. + return true + } + if s.listener == nil { + // Listener not yet bound; conservatively deny cross-origin requests. + return false + } + // The inspector binds to a local address (e.g., "127.0.0.1:4040"). + // Acceptable origins are http:// and https:// variants of that address. + inspectorAddr := s.listener.Addr().String() + return origin == "http://"+inspectorAddr || origin == "https://"+inspectorAddr } // Start binds to the given address and serves the inspector. @@ -50,7 +76,7 @@ func (s *Server) Start(addr string) error { mux.HandleFunc("/api/clear", s.handleClear) mux.HandleFunc("/", s.handleDashboard) - server := &http.Server{Handler: corsMiddleware(mux)} + server := &http.Server{Handler: corsMiddleware(s, mux)} go server.Serve(listener) s.logger.Info().Str("addr", listener.Addr().String()).Msg("inspector started") return nil @@ -72,16 +98,30 @@ func (s *Server) Close() error { return nil } -func corsMiddleware(next http.Handler) http.Handler { +// corsMiddleware enforces same-origin access on the inspector API. +// It only reflects the CORS allow-origin header when the request's Origin +// matches the inspector's own address, preventing any other website from +// reading tunnel traffic via cross-origin requests (CWE-942). +func corsMiddleware(srv *Server, next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Access-Control-Allow-Origin", "*") - w.Header().Set("Access-Control-Allow-Methods", "GET, POST, OPTIONS") - w.Header().Set("Access-Control-Allow-Headers", "Content-Type") - if r.Method == "OPTIONS" { - w.WriteHeader(204) + if srv.isAllowedOrigin(r) { + origin := r.Header.Get("Origin") + if origin != "" { + // Echo the exact origin back — no wildcard. + w.Header().Set("Access-Control-Allow-Origin", origin) + w.Header().Set("Access-Control-Allow-Methods", "GET, POST, OPTIONS") + w.Header().Set("Access-Control-Allow-Headers", "Content-Type") + w.Header().Set("Vary", "Origin") + } + if r.Method == http.MethodOptions { + w.WriteHeader(http.StatusNoContent) + return + } + next.ServeHTTP(w, r) return } - next.ServeHTTP(w, r) + // Origin present but not the inspector's own address — reject. + http.Error(w, "Forbidden", http.StatusForbidden) }) }