From 93bc1b72e3cd566e6447ad7a1f832474aad5dfcc Mon Sep 17 00:00:00 2001
From: Mohammed Al Sahaf <msaa1990@gmail.com>
Date: Tue, 12 Nov 2019 01:33:38 +0300
Subject: core: Use port ranges to avoid OOM with bad inputs (#2859)

* fix OOM issue caught by fuzzing

* use ParsedAddress as the struct name for the result of ParseNetworkAddress

* simplify code using the ParsedAddress type

* minor cleanups
---
 modules/caddyhttp/reverseproxy/healthchecks.go | 10 +++++-----
 modules/caddyhttp/reverseproxy/hosts.go        | 24 ++++++++----------------
 2 files changed, 13 insertions(+), 21 deletions(-)

(limited to 'modules/caddyhttp/reverseproxy')

diff --git a/modules/caddyhttp/reverseproxy/healthchecks.go b/modules/caddyhttp/reverseproxy/healthchecks.go
index 56e97bc..92b3547 100644
--- a/modules/caddyhttp/reverseproxy/healthchecks.go
+++ b/modules/caddyhttp/reverseproxy/healthchecks.go
@@ -102,7 +102,7 @@ func (h *Handler) doActiveHealthChecksForAllHosts() {
 		host := value.(Host)
 
 		go func(networkAddr string, host Host) {
-			network, addrs, err := caddy.ParseNetworkAddress(networkAddr)
+			addr, err := caddy.ParseNetworkAddress(networkAddr)
 			if err != nil {
 				h.HealthChecks.Active.logger.Error("bad network address",
 					zap.String("address", networkAddr),
@@ -110,20 +110,20 @@ func (h *Handler) doActiveHealthChecksForAllHosts() {
 				)
 				return
 			}
-			if len(addrs) != 1 {
+			if addr.PortRangeSize() != 1 {
 				h.HealthChecks.Active.logger.Error("multiple addresses (upstream must map to only one address)",
 					zap.String("address", networkAddr),
 				)
 				return
 			}
-			hostAddr := addrs[0]
-			if network == "unix" || network == "unixgram" || network == "unixpacket" {
+			hostAddr := addr.JoinHostPort(0)
+			if addr.Network == "unix" || addr.Network == "unixgram" || addr.Network == "unixpacket" {
 				// this will be used as the Host portion of a http.Request URL, and
 				// paths to socket files would produce an error when creating URL,
 				// so use a fake Host value instead; unix sockets are usually local
 				hostAddr = "localhost"
 			}
-			err = h.doActiveHealthCheck(DialInfo{Network: network, Address: addrs[0]}, hostAddr, host)
+			err = h.doActiveHealthCheck(DialInfo{Network: addr.Network, Address: hostAddr}, hostAddr, host)
 			if err != nil {
 				h.HealthChecks.Active.logger.Error("active health check failed",
 					zap.String("address", networkAddr),
diff --git a/modules/caddyhttp/reverseproxy/hosts.go b/modules/caddyhttp/reverseproxy/hosts.go
index a16bed0..8bad7c2 100644
--- a/modules/caddyhttp/reverseproxy/hosts.go
+++ b/modules/caddyhttp/reverseproxy/hosts.go
@@ -16,8 +16,7 @@ package reverseproxy
 
 import (
 	"fmt"
-	"net"
-	"strings"
+	"strconv"
 	"sync/atomic"
 
 	"github.com/caddyserver/caddy/v2"
@@ -193,27 +192,20 @@ func (di DialInfo) String() string {
 // the given Replacer. Note that the returned value is not a pointer.
 func fillDialInfo(upstream *Upstream, repl caddy.Replacer) (DialInfo, error) {
 	dial := repl.ReplaceAll(upstream.Dial, "")
-	netw, addrs, err := caddy.ParseNetworkAddress(dial)
+	addr, err := caddy.ParseNetworkAddress(dial)
 	if err != nil {
 		return DialInfo{}, fmt.Errorf("upstream %s: invalid dial address %s: %v", upstream.Dial, dial, err)
 	}
-	if len(addrs) != 1 {
+	if numPorts := addr.PortRangeSize(); numPorts != 1 {
 		return DialInfo{}, fmt.Errorf("upstream %s: dial address must represent precisely one socket: %s represents %d",
-			upstream.Dial, dial, len(addrs))
-	}
-	var dialHost, dialPort string
-	if !strings.Contains(netw, "unix") {
-		dialHost, dialPort, err = net.SplitHostPort(addrs[0])
-		if err != nil {
-			dialHost = addrs[0] // assume there was no port
-		}
+			upstream.Dial, dial, numPorts)
 	}
 	return DialInfo{
 		Upstream: upstream,
-		Network:  netw,
-		Address:  addrs[0],
-		Host:     dialHost,
-		Port:     dialPort,
+		Network:  addr.Network,
+		Address:  addr.JoinHostPort(0),
+		Host:     addr.Host,
+		Port:     strconv.Itoa(int(addr.StartPort)),
 	}, nil
 }
 
-- 
cgit v1.2.3