From b6bd6b951ee44471530bb9141fb5aacd695b4018 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Fri, 24 Apr 2026 11:28:55 +0200 Subject: [PATCH] Release js.FuncOf callbacks in wasm ssh and rdp to prevent leaks --- client/wasm/internal/rdp/cert_validation.go | 16 ++- client/wasm/internal/rdp/rdcleanpath.go | 105 +++++++++++++++----- client/wasm/internal/ssh/handlers.go | 27 +++-- 3 files changed, 109 insertions(+), 39 deletions(-) diff --git a/client/wasm/internal/rdp/cert_validation.go b/client/wasm/internal/rdp/cert_validation.go index 1678c3996..9b1679087 100644 --- a/client/wasm/internal/rdp/cert_validation.go +++ b/client/wasm/internal/rdp/cert_validation.go @@ -46,17 +46,23 @@ func (p *RDCleanPathProxy) validateCertificateWithJS(conn *proxyConnection, cert promise := conn.wsHandlers.Call("onCertificateRequest", certInfo) - resultChan := make(chan bool) - errorChan := make(chan error) + resultChan := make(chan bool, 1) + errorChan := make(chan error, 1) - promise.Call("then", js.FuncOf(func(this js.Value, args []js.Value) interface{} { + thenFn := js.FuncOf(func(this js.Value, args []js.Value) interface{} { result := args[0].Bool() resultChan <- result return nil - })).Call("catch", js.FuncOf(func(this js.Value, args []js.Value) interface{} { + }) + defer thenFn.Release() + + catchFn := js.FuncOf(func(this js.Value, args []js.Value) interface{} { errorChan <- fmt.Errorf("certificate validation failed") return nil - })) + }) + defer catchFn.Release() + + promise.Call("then", thenFn).Call("catch", catchFn) select { case result := <-resultChan: diff --git a/client/wasm/internal/rdp/rdcleanpath.go b/client/wasm/internal/rdp/rdcleanpath.go index 16bf63bb9..81540fad7 100644 --- a/client/wasm/internal/rdp/rdcleanpath.go +++ b/client/wasm/internal/rdp/rdcleanpath.go @@ -11,6 +11,7 @@ import ( "io" "net" "sync" + "sync/atomic" "syscall/js" "time" @@ -57,6 +58,8 @@ type RDCleanPathProxy struct { } activeConnections map[string]*proxyConnection destinations map[string]string + pendingHandlers map[string]js.Func + nextID atomic.Uint64 mu sync.Mutex } @@ -66,8 +69,15 @@ type proxyConnection struct { rdpConn net.Conn tlsConn *tls.Conn wsHandlers js.Value - ctx context.Context - cancel context.CancelFunc + // Go-side callbacks exposed to JS. js.FuncOf pins the Go closure in a + // global handle map and MUST be released, otherwise every connection + // leaks the Go memory the closure captures. + wsHandlerFn js.Func + onMessageFn js.Func + onCloseFn js.Func + cleanupOnce sync.Once + ctx context.Context + cancel context.CancelFunc } // NewRDCleanPathProxy creates a new RDCleanPath proxy @@ -80,7 +90,11 @@ func NewRDCleanPathProxy(client interface { } } -// CreateProxy creates a new proxy endpoint for the given destination +// CreateProxy creates a new proxy endpoint for the given destination. +// The registered handler fn and its destinations/pendingHandlers entries are +// only released once a connection is established and cleanupConnection runs. +// If a caller invokes CreateProxy but never connects to the returned URL, +// those entries stay pinned for the lifetime of the page. func (p *RDCleanPathProxy) CreateProxy(hostname, port string) js.Value { destination := fmt.Sprintf("%s:%s", hostname, port) @@ -88,7 +102,7 @@ func (p *RDCleanPathProxy) CreateProxy(hostname, port string) js.Value { resolve := args[0] go func() { - proxyID := fmt.Sprintf("proxy_%d", len(p.activeConnections)) + proxyID := fmt.Sprintf("proxy_%d", p.nextID.Add(1)) p.mu.Lock() if p.destinations == nil { @@ -100,7 +114,7 @@ func (p *RDCleanPathProxy) CreateProxy(hostname, port string) js.Value { proxyURL := fmt.Sprintf("%s://%s/%s", RDCleanPathProxyScheme, RDCleanPathProxyHost, proxyID) // Register the WebSocket handler for this specific proxy - js.Global().Set(fmt.Sprintf("handleRDCleanPathWebSocket_%s", proxyID), js.FuncOf(func(_ js.Value, args []js.Value) any { + handlerFn := js.FuncOf(func(_ js.Value, args []js.Value) any { if len(args) < 1 { return js.ValueOf("error: requires WebSocket argument") } @@ -108,7 +122,14 @@ func (p *RDCleanPathProxy) CreateProxy(hostname, port string) js.Value { ws := args[0] p.HandleWebSocketConnection(ws, proxyID) return nil - })) + }) + p.mu.Lock() + if p.pendingHandlers == nil { + p.pendingHandlers = make(map[string]js.Func) + } + p.pendingHandlers[proxyID] = handlerFn + p.mu.Unlock() + js.Global().Set(fmt.Sprintf("handleRDCleanPathWebSocket_%s", proxyID), handlerFn) log.Infof("Created RDCleanPath proxy endpoint: %s for destination: %s", proxyURL, destination) resolve.Invoke(proxyURL) @@ -142,6 +163,10 @@ func (p *RDCleanPathProxy) HandleWebSocketConnection(ws js.Value, proxyID string p.mu.Lock() p.activeConnections[proxyID] = conn + if fn, ok := p.pendingHandlers[proxyID]; ok { + conn.wsHandlerFn = fn + delete(p.pendingHandlers, proxyID) + } p.mu.Unlock() p.setupWebSocketHandlers(ws, conn) @@ -150,7 +175,7 @@ func (p *RDCleanPathProxy) HandleWebSocketConnection(ws js.Value, proxyID string } func (p *RDCleanPathProxy) setupWebSocketHandlers(ws js.Value, conn *proxyConnection) { - ws.Set("onGoMessage", js.FuncOf(func(this js.Value, args []js.Value) any { + conn.onMessageFn = js.FuncOf(func(this js.Value, args []js.Value) any { if len(args) < 1 { return nil } @@ -158,13 +183,15 @@ func (p *RDCleanPathProxy) setupWebSocketHandlers(ws js.Value, conn *proxyConnec data := args[0] go p.handleWebSocketMessage(conn, data) return nil - })) + }) + ws.Set("onGoMessage", conn.onMessageFn) - ws.Set("onGoClose", js.FuncOf(func(_ js.Value, args []js.Value) any { + conn.onCloseFn = js.FuncOf(func(_ js.Value, args []js.Value) any { log.Debug("WebSocket closed by JavaScript") conn.cancel() return nil - })) + }) + ws.Set("onGoClose", conn.onCloseFn) } func (p *RDCleanPathProxy) handleWebSocketMessage(conn *proxyConnection, data js.Value) { @@ -261,25 +288,49 @@ func (p *RDCleanPathProxy) handleDirectRDP(conn *proxyConnection, firstPacket [] } func (p *RDCleanPathProxy) cleanupConnection(conn *proxyConnection) { - log.Debugf("Cleaning up connection %s", conn.id) - conn.cancel() - if conn.tlsConn != nil { - log.Debug("Closing TLS connection") - if err := conn.tlsConn.Close(); err != nil { - log.Debugf("Error closing TLS connection: %v", err) + conn.cleanupOnce.Do(func() { + log.Debugf("Cleaning up connection %s", conn.id) + conn.cancel() + if conn.tlsConn != nil { + log.Debug("Closing TLS connection") + if err := conn.tlsConn.Close(); err != nil { + log.Debugf("Error closing TLS connection: %v", err) + } + conn.tlsConn = nil } - conn.tlsConn = nil - } - if conn.rdpConn != nil { - log.Debug("Closing TCP connection") - if err := conn.rdpConn.Close(); err != nil { - log.Debugf("Error closing TCP connection: %v", err) + if conn.rdpConn != nil { + log.Debug("Closing TCP connection") + if err := conn.rdpConn.Close(); err != nil { + log.Debugf("Error closing TCP connection: %v", err) + } + conn.rdpConn = nil } - conn.rdpConn = nil - } - p.mu.Lock() - delete(p.activeConnections, conn.id) - p.mu.Unlock() + js.Global().Delete(fmt.Sprintf("handleRDCleanPathWebSocket_%s", conn.id)) + + // Detach before releasing so late JS calls surface as TypeError instead + // of silent "call to released function". + if conn.wsHandlers.Truthy() { + conn.wsHandlers.Set("onGoMessage", js.Undefined()) + conn.wsHandlers.Set("onGoClose", js.Undefined()) + } + + // wsHandlerFn may be zero-value if the pending handler lookup missed. + if conn.wsHandlerFn.Truthy() { + conn.wsHandlerFn.Release() + } + if conn.onMessageFn.Truthy() { + conn.onMessageFn.Release() + } + if conn.onCloseFn.Truthy() { + conn.onCloseFn.Release() + } + + p.mu.Lock() + delete(p.activeConnections, conn.id) + delete(p.destinations, conn.id) + delete(p.pendingHandlers, conn.id) + p.mu.Unlock() + }) } func (p *RDCleanPathProxy) sendToWebSocket(conn *proxyConnection, data []byte) { diff --git a/client/wasm/internal/ssh/handlers.go b/client/wasm/internal/ssh/handlers.go index ea64eb0aa..6d33916a5 100644 --- a/client/wasm/internal/ssh/handlers.go +++ b/client/wasm/internal/ssh/handlers.go @@ -13,7 +13,7 @@ import ( func CreateJSInterface(client *Client) js.Value { jsInterface := js.Global().Get("Object").Call("create", js.Null()) - jsInterface.Set("write", js.FuncOf(func(this js.Value, args []js.Value) any { + writeFunc := js.FuncOf(func(this js.Value, args []js.Value) any { if len(args) < 1 { return js.ValueOf(false) } @@ -32,9 +32,10 @@ func CreateJSInterface(client *Client) js.Value { _, err := client.Write(bytes) return js.ValueOf(err == nil) - })) + }) + jsInterface.Set("write", writeFunc) - jsInterface.Set("resize", js.FuncOf(func(this js.Value, args []js.Value) any { + resizeFunc := js.FuncOf(func(this js.Value, args []js.Value) any { if len(args) < 2 { return js.ValueOf(false) } @@ -42,14 +43,26 @@ func CreateJSInterface(client *Client) js.Value { rows := args[1].Int() err := client.Resize(cols, rows) return js.ValueOf(err == nil) - })) + }) + jsInterface.Set("resize", resizeFunc) - jsInterface.Set("close", js.FuncOf(func(this js.Value, args []js.Value) any { + closeFunc := js.FuncOf(func(this js.Value, args []js.Value) any { client.Close() return js.Undefined() - })) + }) + jsInterface.Set("close", closeFunc) - go readLoop(client, jsInterface) + go func() { + readLoop(client, jsInterface) + // Detach before releasing so late JS calls surface as TypeError instead + // of silent "call to released function". + jsInterface.Set("write", js.Undefined()) + jsInterface.Set("resize", js.Undefined()) + jsInterface.Set("close", js.Undefined()) + writeFunc.Release() + resizeFunc.Release() + closeFunc.Release() + }() return jsInterface }