GitLab

The client lib.Handler function does not close pconn and sess in the event that sess.OpenStream() returns an error. This is a minor resource leak.

The pconn.Close() and sess.Close() statements should be moved up and placed in a defer.

This is issue UCB-02-001 from the 2021 security audit of Turbo Tunnel by Cure53. Quoted below:

UCB-02-001 WP1: Memory leak in Handler() routine of Snowflake client lib (Low)

During a review of the Snowflake client library, the discovery was made that the Handler() function - responsible for establishing a WebRTC connection to the remote peer - does not correctly close the connection and established smux session in the eventuality that a stream cannot be opened. This could result in a memory leak on the Snowflake client side, as well as a resource leak on the server side of the connection.

Affected file: snowflake/client/lib/snowflake.go

Affected code:

func Handler(socks net.Conn, tongue Tongue) error {
        [...]
        // Create a new smux session
        log.Printf("---- Handler: starting a new session ---")
        pconn, sess, err := newSession(snowflakes)
        if err != nil {
                return err
        }
        // On the smux session we overlay a stream.
        stream, err := sess.OpenStream()
        if err != nil {
                return err
        }
        [...]
}

It is recommended to close all open connections using defer in order to properly alleviate all allocated resources when the function returns.

---

!31 (closed) restructured things and this issue now applies to Transport.Dial. If newSession fails, then snowflakes leaks; and if sess.OpenStream fails, then snowflakes, pconn, and sess leak. There's also an error at the call site: any error is logged, but then the code goes on to call copyLoop with the nil Conn that was returned.