CVE-2024-34068: Insufficient Information (NVD-CWE-noinfo)
value: Pterodactyl wings is the server control plane for Pterodactyl Panel. An authenticated user who has access to a game server is able to bypass the previously implemented access control (GHSA-6rg3-8h8x-5xfv) that prevents accessing internal endpoints of the node hosting Wings in the pull endpoint. This would allow malicious users to potentially access resources on local networks that would otherwise be inaccessible. This issue has been addressed in version 1.11.2 and users are advised to upgrade. Users unable to upgrade may enable the `api.disable_remote_download` option as a workaround. 
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-05-03T18:15Z
Repository: https://github.com/pterodactyl/wings
Language: Go
Files changed: router/downloader/downloader.go
Commit date: 2024-04-10 15:22:09 -0600
Commit message: downloader: move internal subnet validation into http Transport

Diff: diff --git a/router/downloader/downloader.go b/router/downloader/downloader.go
index d5436d63..e5c6523e 100644
--- a/router/downloader/downloader.go
+++ b/router/downloader/downloader.go
@@ -18,24 +18,62 @@ import (
 	"github.com/google/uuid"
 
 	"github.com/pterodactyl/wings/server"
 )
 
-var client = &http.Client{
-	Timeout: time.Hour * 12,
-	// Disallow any redirect on an HTTP call. This is a security requirement: do not modify
-	// this logic without first ensuring that the new target location IS NOT within the current
-	// instance's local network.
-	//
-	// This specific error response just causes the client to not follow the redirect and
-	// returns the actual redirect response to the caller. Not perfect, but simple and most
-	// people won't be using URLs that redirect anyways hopefully?
-	//
-	// We'll re-evaluate this down the road if needed.
-	CheckRedirect: func(req *http.Request, via []*http.Request) error {
-		return http.ErrUseLastResponse
-	},
+var client *http.Client
+
+func init() {
+	dialer := &net.Dialer{
+		LocalAddr: nil,
+	}
+
+	trnspt := http.DefaultTransport.(*http.Transport).Clone()
+	trnspt.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
+		c, err := dialer.DialContext(ctx, network, addr)
+		if err != nil {
+			return nil, errors.WithStack(err)
+		}
+
+		ipStr, _, err := net.SplitHostPort(c.RemoteAddr().String())
+		if err != nil {
+			return c, errors.WithStack(err)
+		}
+		ip := net.ParseIP(ipStr)
+		if ip == nil {
+			return c, errors.WithStack(ErrInvalidIPAddress)
+		}
+		if ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsInterfaceLocalMulticast() {
+			return c, errors.WithStack(ErrInternalResolution)
+		}
+		for _, block := range internalRanges {
+			if !block.Contains(ip) {
+				continue
+			}
+			return c, errors.WithStack(ErrInternalResolution)
+		}
+		return c, nil
+	}
+
+	client = &http.Client{
+		Timeout: time.Hour * 12,
+
+		Transport: trnspt,
+
+		// Disallow any redirect on an HTTP call. This is a security requirement: do not modify
+		// this logic without first ensuring that the new target location IS NOT within the current
+		// instance's local network.
+		//
+		// This specific error response just causes the client to not follow the redirect and
+		// returns the actual redirect response to the caller. Not perfect, but simple and most
+		// people won't be using URLs that redirect anyways hopefully?
+		//
+		// We'll re-evaluate this down the road if needed.
+		CheckRedirect: func(req *http.Request, via []*http.Request) error {
+			return http.ErrUseLastResponse
+		},
+	}
 }
 
 var instance = &Downloader{
 	// Tracks all the active downloads.
 	downloadCache: make(map[string]*Download),
@@ -141,16 +179,10 @@ func (dl Download) MarshalJSON() ([]byte, error) {
 func (dl *Download) Execute() error {
 	ctx, cancel := context.WithTimeout(context.Background(), time.Hour*12)
 	dl.cancelFunc = &cancel
 	defer dl.Cancel()
 
-	// Always ensure that we're checking the destination for the download to avoid a malicious
-	// user from accessing internal network resources.
-	if err := dl.isExternalNetwork(ctx); err != nil {
-		return err
-	}
-
 	// At this point we have verified the destination is not within the local network, so we can
 	// now make a request to that URL and pull down the file, saving it to the server's data
 	// directory.
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, dl.req.URL.String(), nil)
 	if err != nil {
@@ -241,63 +273,10 @@ func (dl *Download) counter(contentLength int64) *Counter {
 	return &Counter{
 		onWrite: onWrite,
 	}
 }
 
-// Verifies that a given download resolves to a location not within the current local
-// network for the machine. If the final destination of a resource is within the local
-// network an ErrInternalResolution error is returned.
-func (dl *Download) isExternalNetwork(ctx context.Context) error {
-	dialer := &net.Dialer{
-		LocalAddr: nil,
-	}
-
-	host := dl.req.URL.Host
-
-	// This cluster-fuck of math and integer shit converts an integer IP into a proper IPv4.
-	// For example: 16843009 would become 1.1.1.1
-	//if i, err := strconv.ParseInt(host, 10, 64); err == nil {
-	//	host = strconv.FormatInt((i>>24)&0xFF, 10) + "." + strconv.FormatInt((i>>16)&0xFF, 10) + "." + strconv.FormatInt((i>>8)&0xFF, 10) + "." + strconv.FormatInt(i&0xFF, 10)
-	//}
-
-	if _, _, err := net.SplitHostPort(host); err != nil {
-		if !strings.Contains(err.Error(), "missing port in address") {
-			return errors.WithStack(err)
-		}
-		switch dl.req.URL.Scheme {
-		case "http":
-			host += ":80"
-		case "https":
-			host += ":443"
-		}
-	}
-
-	c, err := dialer.DialContext(ctx, "tcp", host)
-	if err != nil {
-		return errors.WithStack(err)
-	}
-	_ = c.Close()
-
-	ipStr, _, err := net.SplitHostPort(c.RemoteAddr().String())
-	if err != nil {
-		return errors.WithStack(err)
-	}
-	ip := net.ParseIP(ipStr)
-	if ip == nil {
-		return errors.WithStack(ErrInvalidIPAddress)
-	}
-	if ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsInterfaceLocalMulticast() {
-		return errors.WithStack(ErrInternalResolution)
-	}
-	for _, block := range internalRanges {
-		if block.Contains(ip) {
-			return errors.WithStack(ErrInternalResolution)
-		}
-	}
-	return nil
-}
-
 // Downloader represents a global downloader that keeps track of all currently processing downloads
 // for the machine.
 type Downloader struct {
 	mu            sync.RWMutex
 	downloadCache map[string]*Download

Vulnerable code: var client = &http.Client{
	Timeout: time.Hour * 12,
	// Disallow any redirect on an HTTP call. This is a security requirement: do not modify
	// this logic without first ensuring that the new target location IS NOT within the current
	// instance's local network.
	//
	// This specific error response just causes the client to not follow the redirect and
	// returns the actual redirect response to the caller. Not perfect, but simple and most
	// people won't be using URLs that redirect anyways hopefully?
	//
	// We'll re-evaluate this down the road if needed.
	CheckRedirect: func(req *http.Request, via []*http.Request) error {
		return http.ErrUseLastResponse
	},
	// Always ensure that we're checking the destination for the download to avoid a malicious
	// user from accessing internal network resources.
	if err := dl.isExternalNetwork(ctx); err != nil {
		return err
	}

// Verifies that a given download resolves to a location not within the current local
// network for the machine. If the final destination of a resource is within the local
// network an ErrInternalResolution error is returned.
func (dl *Download) isExternalNetwork(ctx context.Context) error {
	dialer := &net.Dialer{
		LocalAddr: nil,
	}

	host := dl.req.URL.Host

	// This cluster-fuck of math and integer shit converts an integer IP into a proper IPv4.
	// For example: 16843009 would become 1.1.1.1
	//if i, err := strconv.ParseInt(host, 10, 64); err == nil {
	//	host = strconv.FormatInt((i>>24)&0xFF, 10) + "." + strconv.FormatInt((i>>16)&0xFF, 10) + "." + strconv.FormatInt((i>>8)&0xFF, 10) + "." + strconv.FormatInt(i&0xFF, 10)
	//}

	if _, _, err := net.SplitHostPort(host); err != nil {
		if !strings.Contains(err.Error(), "missing port in address") {
			return errors.WithStack(err)
		}
		switch dl.req.URL.Scheme {
		case "http":
			host += ":80"
		case "https":
			host += ":443"
		}
	}

	c, err := dialer.DialContext(ctx, "tcp", host)
	if err != nil {
		return errors.WithStack(err)
	}
	_ = c.Close()

	ipStr, _, err := net.SplitHostPort(c.RemoteAddr().String())
	if err != nil {
		return errors.WithStack(err)
	}
	ip := net.ParseIP(ipStr)
	if ip == nil {
		return errors.WithStack(ErrInvalidIPAddress)
	}
	if ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsInterfaceLocalMulticast() {
		return errors.WithStack(ErrInternalResolution)
	}
	for _, block := range internalRanges {
		if block.Contains(ip) {
			return errors.WithStack(ErrInternalResolution)
		}
	}
	return nil
}

Fixed code: var client *http.Client

func init() {
	dialer := &net.Dialer{
		LocalAddr: nil,
	}

	trnspt := http.DefaultTransport.(*http.Transport).Clone()
	trnspt.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
		c, err := dialer.DialContext(ctx, network, addr)
		if err != nil {
			return nil, errors.WithStack(err)
		}

		ipStr, _, err := net.SplitHostPort(c.RemoteAddr().String())
		if err != nil {
			return c, errors.WithStack(err)
		}
		ip := net.ParseIP(ipStr)
		if ip == nil {
			return c, errors.WithStack(ErrInvalidIPAddress)
		}
		if ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsInterfaceLocalMulticast() {
			return c, errors.WithStack(ErrInternalResolution)
		}
		for _, block := range internalRanges {
			if !block.Contains(ip) {
				continue
			}
			return c, errors.WithStack(ErrInternalResolution)
		}
		return c, nil
	}

	client = &http.Client{
		Timeout: time.Hour * 12,

		Transport: trnspt,

		// Disallow any redirect on an HTTP call. This is a security requirement: do not modify
		// this logic without first ensuring that the new target location IS NOT within the current
		// instance's local network.
		//
		// This specific error response just causes the client to not follow the redirect and
		// returns the actual redirect response to the caller. Not perfect, but simple and most
		// people won't be using URLs that redirect anyways hopefully?
		//
		// We'll re-evaluate this down the road if needed.
		CheckRedirect: func(req *http.Request, via []*http.Request) error {
			return http.ErrUseLastResponse
		},
	}