Skip to content
GitLab
  • Menu
Projects Groups Snippets
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in
  • S Snowflake
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 71
    • Issues 71
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 4
    • Merge requests 4
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • The Tor Project
  • Anti-censorship
  • Pluggable Transports
  • Snowflake
  • Issues
  • #40042
Closed
Open
Created Apr 20, 2021 by David Fifield@dcfOwner

Client Transport.Dial fails to release resources on error

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.

Edited May 24, 2021 by David Fifield
Assignee
Assign to
Time tracking