Prohibit connecting to loopback, link-local multicast, and link-local unicast IP addresses by default

Author: DarthSim

CHANGELOG.md

@@ -1,6 +1,11 @@
 # Changelog
 
 ## [Unreleased]
+### Add
+- Add the `IMGPROXY_ALLOW_LOOPBACK_SOURCE_ADDRESSES`, `IMGPROXY_ALLOW_LINK_LOCAL_SOURCE_ADDRESSES`, and `IMGPROXY_ALLOW_PRIVATE_SOURCE_ADDRESSES` configs.
+
+### Change
+- Connecting to loopback, link-local multicast, and link-local unicast IP addresses when requesting source images is prohibited by default.
 
 ## [3.14.0] - 2023-03-07
 ## Add

config/config.go

@@ -85,7 +85,10 @@ var (
 	IgnoreSslVerification bool
 	DevelopmentErrorsMode bool
 
-	AllowedSources []*regexp.Regexp
+	AllowedSources                []*regexp.Regexp
+	AllowLoopbackSourceAddresses  bool
+	AllowLinkLocalSourceAddresses bool
+	AllowPrivateSourceAddresses   bool
 
 	SanitizeSvg bool
 
@@ -275,6 +278,9 @@ func Reset() {
 	DevelopmentErrorsMode = false
 
 	AllowedSources = make([]*regexp.Regexp, 0)
+	AllowLoopbackSourceAddresses = false
+	AllowLinkLocalSourceAddresses = false
+	AllowPrivateSourceAddresses = true
 
 	SanitizeSvg = true
 
@@ -404,6 +410,9 @@ func Configure() error {
 	configurators.Int(&MaxRedirects, "IMGPROXY_MAX_REDIRECTS")
 
 	configurators.Patterns(&AllowedSources, "IMGPROXY_ALLOWED_SOURCES")
+	configurators.Bool(&AllowLoopbackSourceAddresses, "IMGPROXY_ALLOW_LOOPBACK_SOURCE_ADDRESSES")
+	configurators.Bool(&AllowLinkLocalSourceAddresses, "IMGPROXY_ALLOW_LINK_LOCAL_SOURCE_ADDRESSES")
+	configurators.Bool(&AllowPrivateSourceAddresses, "IMGPROXY_ALLOW_PRIVATE_SOURCE_ADDRESSES")
 
 	configurators.Bool(&SanitizeSvg, "IMGPROXY_SANITIZE_SVG")
 

docs/configuration.md

@@ -102,7 +102,11 @@ You can limit allowed source URLs with the following variable:
 
   ✅ Good: `http://example.com/`
 
-If the trailing slash is absent, `http://example.com@baddomain.com` would be a permissable URL, however, the request would be made to `baddomain.com`.
+  If the trailing slash is absent, `http://example.com@baddomain.com` would be a permissable URL, however, the request would be made to `baddomain.com`.
+
+* `IMGPROXY_ALLOW_LOOPBACK_SOURCE_ADDRESSES`: when `true`, allows connecting to loopback IP addresses (`127.0.0.1`-`127.255.255.255` and IPv6 analogues) when requesting source images. Default: `false`
+* `IMGPROXY_ALLOW_LINK_LOCAL_SOURCE_ADDRESSES`:  when `true`, allows connecting to link-local multicast and unicast IP addresses (`224.0.0.1`-`224.0.0.255`, `169.254.0.1`-`169.254.255.255`, and IPv6 analogues) when requesting source images. Default: `false`
+* `IMGPROXY_ALLOW_PRIVATE_SOURCE_ADDRESSES`: when `true`, allows connecting to private IP addresses (`10.0.0.0 - 10.255.255.255`, `172.16.0.0 - 172.31.255.255`, `192.168.0.0 - 192.168.255.255`, and IPv6 analogues) when requesting source images. Default: `true`
 
 * `IMGPROXY_SANITIZE_SVG`: when `true`, imgproxy will remove scripts from SVG images to prevent XSS attacks. Defaut: `true`
 

imagedata/download.go

@@ -9,6 +9,7 @@ import (
 	"net"
 	"net/http"
 	"net/http/cookiejar"
+	"syscall"
 	"time"
 
 	"github.com/imgproxy/imgproxy/v3/config"
@@ -63,6 +64,9 @@ func initDownloading() error {
 			Timeout:   30 * time.Second,
 			KeepAlive: 30 * time.Second,
 			DualStack: true,
+			Control: func(network, address string, c syscall.RawConn) error {
+				return security.VerifySourceNetwork(address)
+			},
 		}).DialContext,
 		MaxIdleConns:          100,
 		MaxIdleConnsPerHost:   config.Concurrency + 1,

imagedata/error.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 
 	"github.com/imgproxy/imgproxy/v3/ierrors"
+	"github.com/imgproxy/imgproxy/v3/security"
 )
 
 type httpError interface {
@@ -16,16 +17,25 @@ type httpError interface {
 func wrapError(err error) error {
 	isTimeout := false
 
-	if errors.Is(err, context.Canceled) {
+	switch {
+	case errors.Is(err, context.DeadlineExceeded):
+		isTimeout = true
+	case errors.Is(err, context.Canceled):
 		return ierrors.New(
 			499,
 			fmt.Sprintf("The image request is cancelled: %s", err),
 			msgSourceImageIsUnreachable,
 		)
-	} else if errors.Is(err, context.DeadlineExceeded) {
-		isTimeout = true
-	} else if httpErr, ok := err.(httpError); ok {
-		isTimeout = httpErr.Timeout()
+	case errors.Is(err, security.ErrSourceAddressNotAllowed), errors.Is(err, security.ErrInvalidSourceAddress):
+		return ierrors.New(
+			404,
+			err.Error(),
+			msgSourceImageIsUnreachable,
+		)
+	default:
+		if httpErr, ok := err.(httpError); ok {
+			isTimeout = httpErr.Timeout()
+		}
 	}
 
 	if !isTimeout {

processing_handler.go

@@ -233,13 +233,8 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) {
 	po, imageURL, err := options.ParsePath(path, r.Header)
 	checkErr(ctx, "path_parsing", err)
 
-	if !security.VerifySourceURL(imageURL) {
-		sendErrAndPanic(ctx, "security", ierrors.New(
-			404,
-			fmt.Sprintf("Source URL is not allowed: %s", imageURL),
-			"Invalid source",
-		))
-	}
+	err = security.VerifySourceURL(imageURL)
+	checkErr(ctx, "security", err)
 
 	if po.Raw {
 		streamOriginImage(ctx, reqID, r, rw, po, imageURL)

processing_handler_test.go

@@ -40,6 +40,8 @@ func (s *ProcessingHandlerTestSuite) SetupSuite() {
 	require.Nil(s.T(), err)
 
 	config.LocalFileSystemRoot = filepath.Join(wd, "/testdata")
+	// Disable keep-alive to test connection restrictions
+	config.ClientKeepAliveTimeout = 0
 
 	err = initialize()
 	require.Nil(s.T(), err)
@@ -58,6 +60,7 @@ func (s *ProcessingHandlerTestSuite) SetupTest() {
 	// We don't need config.LocalFileSystemRoot anymore as it is used
 	// only during initialization
 	config.Reset()
+	config.AllowLoopbackSourceAddresses = true
 }
 
 func (s *ProcessingHandlerTestSuite) send(path string, header ...http.Header) *httptest.ResponseRecorder {
@@ -210,6 +213,28 @@ func (s *ProcessingHandlerTestSuite) TestSourceValidation() {
 	}
 }
 
+func (s *ProcessingHandlerTestSuite) TestSourceNetworkValidation() {
+	data := s.readTestFile("test1.png")
+
+	server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
+		rw.WriteHeader(200)
+		rw.Write(data)
+	}))
+	defer server.Close()
+
+	var rw *httptest.ResponseRecorder
+
+	u := fmt.Sprintf("/unsafe/rs:fill:4:4/plain/%s/test1.png", server.URL)
+	fmt.Println(u)
+
+	rw = s.send(u)
+	require.Equal(s.T(), 200, rw.Result().StatusCode)
+
+	config.AllowLoopbackSourceAddresses = false
+	rw = s.send(u)
+	require.Equal(s.T(), 404, rw.Result().StatusCode)
+}
+
 func (s *ProcessingHandlerTestSuite) TestSourceFormatNotSupported() {
 	vips.DisableLoadSupport(imagetype.PNG)
 	defer vips.ResetLoadSupport()

security/source.go

@@ -1,17 +1,57 @@
 package security
 
 import (
+	"errors"
+	"fmt"
+	"net"
+
 	"github.com/imgproxy/imgproxy/v3/config"
+	"github.com/imgproxy/imgproxy/v3/ierrors"
 )
 
-func VerifySourceURL(imageURL string) bool {
+var ErrSourceAddressNotAllowed = errors.New("source address is not allowed")
+var ErrInvalidSourceAddress = errors.New("invalid source address")
+
+func VerifySourceURL(imageURL string) error {
 	if len(config.AllowedSources) == 0 {
-		return true
+		return nil
 	}
+
 	for _, allowedSource := range config.AllowedSources {
 		if allowedSource.MatchString(imageURL) {
-			return true
+			return nil
 		}
 	}
-	return false
+
+	return ierrors.New(
+		404,
+		fmt.Sprintf("Source URL is not allowed: %s", imageURL),
+		"Invalid source",
+	)
+}
+
+func VerifySourceNetwork(addr string) error {
+	host, _, err := net.SplitHostPort(addr)
+	if err != nil {
+		host = addr
+	}
+
+	ip := net.ParseIP(host)
+	if ip == nil {
+		return ErrInvalidSourceAddress
+	}
+
+	if !config.AllowLoopbackSourceAddresses && ip.IsLoopback() {
+		return ErrSourceAddressNotAllowed
+	}
+
+	if !config.AllowLinkLocalSourceAddresses && (ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast()) {
+		return ErrSourceAddressNotAllowed
+	}
+
+	if !config.AllowPrivateSourceAddresses && ip.IsPrivate() {
+		return ErrSourceAddressNotAllowed
+	}
+
+	return nil
 }