Security: limit data read from external sources (#3076)

Author: reinkrul

auth/client/iam/client.go

@@ -70,7 +70,7 @@ func (hb HTTPClient) OAuthAuthorizationServerMetadata(ctx context.Context, webDI
 	var metadata oauth.AuthorizationServerMetadata
 	var data []byte
 
-	if data, err = io.ReadAll(response.Body); err != nil {
+	if data, err = core.LimitedReadAll(response.Body); err != nil {
 		return nil, fmt.Errorf("unable to read response: %w", err)
 	}
 	if err = json.Unmarshal(data, &metadata); err != nil {
@@ -165,7 +165,7 @@ func (hb HTTPClient) AccessToken(ctx context.Context, tokenEndpoint string, data
 	}
 
 	var responseData []byte
-	if responseData, err = io.ReadAll(response.Body); err != nil {
+	if responseData, err = core.LimitedReadAll(response.Body); err != nil {
 		return token, fmt.Errorf("unable to read response: %w", err)
 	}
 	if err = json.Unmarshal(responseData, &token); err != nil {
@@ -202,7 +202,6 @@ func (hb HTTPClient) PostAuthorizationResponse(ctx context.Context, vp vc.Verifi
 }
 
 func (hb HTTPClient) OpenIdConfiguration(ctx context.Context, serverURL string) (*oauth.OpenIDConfigurationMetadata, error) {
-
 	metadataURL, err := oauth.IssuerIdToWellKnown(serverURL, oauth.OpenIdConfigurationWellKnown, hb.strictMode)
 	if err != nil {
 		return nil, err
@@ -224,7 +223,7 @@ func (hb HTTPClient) OpenIdConfiguration(ctx context.Context, serverURL string)
 	var metadata oauth.OpenIDConfigurationMetadata
 	var data []byte
 
-	if data, err = io.ReadAll(response.Body); err != nil {
+	if data, err = core.LimitedReadAll(response.Body); err != nil {
 		return nil, fmt.Errorf("unable to read response: %w", err)
 	}
 	if err = json.Unmarshal(data, &metadata); err != nil {
@@ -261,7 +260,7 @@ func (hb HTTPClient) OpenIdCredentialIssuerMetadata(ctx context.Context, webDID
 	var metadata oauth.OpenIDCredentialIssuerMetadata
 	var data []byte
 
-	if data, err = io.ReadAll(response.Body); err != nil {
+	if data, err = core.LimitedReadAll(response.Body); err != nil {
 		return nil, fmt.Errorf("unable to read response: %w", err)
 	}
 	if err = json.Unmarshal(data, &metadata); err != nil {
@@ -300,7 +299,7 @@ func (hb HTTPClient) AccessTokenOid4vci(ctx context.Context, presentationDefinit
 	}
 
 	var responseData []byte
-	if responseData, err = io.ReadAll(response.Body); err != nil {
+	if responseData, err = core.LimitedReadAll(response.Body); err != nil {
 		return nil, fmt.Errorf("unable to read response: %w", err)
 	}
 
@@ -406,7 +405,7 @@ func (hb HTTPClient) doRequest(ctx context.Context, request *http.Request, targe
 
 	var data []byte
 
-	if data, err = io.ReadAll(response.Body); err != nil {
+	if data, err = core.LimitedReadAll(response.Body); err != nil {
 		return fmt.Errorf("unable to read response: %w", err)
 	}
 	if err = json.Unmarshal(data, &target); err != nil {

core/http_client.go

@@ -34,6 +34,21 @@ import (
 // If the response body is longer than this, it will be truncated.
 const HttpResponseBodyLogClipAt = 200
 
+// DefaultMaxHttpResponseSize is a default maximum size of an HTTP response body that will be read.
+// Very large or unbounded HTTP responses can cause denial-of-service, so it's good to limit how much data is read.
+// This of course heavily depends on the use case, but 1MB is a reasonable default.
+const DefaultMaxHttpResponseSize = 1024 * 1024
+
+// LimitedReadAll reads the given reader until the DefaultMaxHttpResponseSize is reached.
+// It returns an error if more data is available than DefaultMaxHttpResponseSize.
+func LimitedReadAll(reader io.Reader) ([]byte, error) {
+	result, err := io.ReadAll(io.LimitReader(reader, DefaultMaxHttpResponseSize+1))
+	if len(result) > DefaultMaxHttpResponseSize {
+		return nil, fmt.Errorf("data to read exceeds max. safety limit of %d bytes", DefaultMaxHttpResponseSize)
+	}
+	return result, err
+}
+
 // HttpError describes an error returned when invoking a remote server.
 type HttpError struct {
 	error
@@ -51,7 +66,7 @@ func TestResponseCode(expectedStatusCode int, response *http.Response) error {
 // It logs using the given logger, unless nil is passed.
 func TestResponseCodeWithLog(expectedStatusCode int, response *http.Response, log *logrus.Entry) error {
 	if response.StatusCode != expectedStatusCode {
-		responseData, _ := io.ReadAll(response.Body)
+		responseData, _ := LimitedReadAll(response.Body)
 		if log != nil {
 			// Cut off the response body to 100 characters max to prevent logging of large responses
 			responseBodyString := string(responseData)

core/http_client_test.go

@@ -25,7 +25,7 @@ import (
 	"github.com/sirupsen/logrus/hooks/test"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	io2 "io"
+	"io"
 	stdHttp "net/http"
 	"net/http/httptest"
 	"net/url"
@@ -162,7 +162,7 @@ type readCloser []byte
 
 func (r readCloser) Read(p []byte) (n int, err error) {
 	copy(p, r)
-	return len(r), io2.EOF
+	return len(r), io.EOF
 }
 
 func (r readCloser) Close() error {
@@ -174,3 +174,20 @@ func newErrorTokenBuilder() AuthorizationTokenGenerator {
 		return "", errors.New("error")
 	}
 }
+
+func TestLimitedReadAll(t *testing.T) {
+	t.Run("less than limit", func(t *testing.T) {
+		data := strings.Repeat("a", 10)
+		result, err := LimitedReadAll(strings.NewReader(data))
+
+		assert.NoError(t, err)
+		assert.Equal(t, []byte(data), result)
+	})
+	t.Run("more than limit", func(t *testing.T) {
+		data := strings.Repeat("a", DefaultMaxHttpResponseSize+1)
+		result, err := LimitedReadAll(strings.NewReader(data))
+
+		assert.EqualError(t, err, "data to read exceeds max. safety limit of 1048576 bytes")
+		assert.Nil(t, result)
+	})
+}

vcr/openid4vci/issuer_client.go

@@ -28,7 +28,6 @@ import (
 	"github.com/nuts-foundation/nuts-node/auth/oauth"
 	"github.com/nuts-foundation/nuts-node/core"
 	"github.com/nuts-foundation/nuts-node/vcr/log"
-	"io"
 	"net/http"
 	"net/http/httptrace"
 	"net/url"
@@ -168,7 +167,7 @@ func httpDo(httpClient core.HTTPRequestDoer, httpRequest *http.Request, result i
 		return fmt.Errorf("http request error: %w", err)
 	}
 	defer httpResponse.Body.Close()
-	responseBody, err := io.ReadAll(httpResponse.Body)
+	responseBody, err := core.LimitedReadAll(httpResponse.Body)
 	if err != nil {
 		return fmt.Errorf("read error (%s): %w", httpRequest.URL, err)
 	}

vcr/revocation/statuslist2021_verifier.go

@@ -22,7 +22,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"io"
 	"net/http"
 	"strconv"
 	"time"
@@ -199,7 +198,7 @@ func (cs *StatusList2021) download(statusListCredential string) (*vc.VerifiableC
 				Debug("Failed to close response body")
 		}
 	}()
-	body, err := io.ReadAll(res.Body)
+	body, err := core.LimitedReadAll(res.Body) // default minimum size is 16kb (PII entropy), so 1mb is already unlikely
 	if res.StatusCode > 299 || err != nil {
 		return nil, errors.Join(fmt.Errorf("fetching StatusList2021Credential from '%s' failed", statusListCredential), err)
 	}

vdr/didweb/web.go

@@ -23,8 +23,8 @@ import (
 	"errors"
 	"fmt"
 	"github.com/nuts-foundation/go-did/did"
+	"github.com/nuts-foundation/nuts-node/core"
 	"github.com/nuts-foundation/nuts-node/vdr/resolver"
-	"io"
 	"mime"
 	"net/http"
 	"time"
@@ -107,7 +107,7 @@ func (w Resolver) Resolve(id did.DID, _ *resolver.ResolveMetadata) (*did.Documen
 	}
 
 	// Read document
-	data, err := io.ReadAll(httpResponse.Body)
+	data, err := core.LimitedReadAll(httpResponse.Body)
 	if err != nil {
 		return nil, nil, fmt.Errorf("did:web HTTP response read error: %w", err)
 	}