Merge pull request #5050 from tamirms/internal-fixes

horizon: Merge internal fixes into master

gxdr/validator.go

@@ -0,0 +1,70 @@
+package gxdr
+
+import (
+	"encoding/base64"
+	"strings"
+
+	goxdr "github.com/xdrpp/goxdr/xdr"
+)
+
+const DefaultMaxDepth = 500
+
+type depthLimiter struct {
+	depth    int
+	maxDepth int
+	decoder  *goxdr.XdrIn
+}
+
+func (*depthLimiter) Sprintf(f string, args ...interface{}) string {
+	return ""
+}
+
+func (d *depthLimiter) Marshal(field string, i goxdr.XdrType) {
+	switch t := goxdr.XdrBaseType(i).(type) {
+	case goxdr.XdrAggregate:
+		if d.depth > d.maxDepth {
+			goxdr.XdrPanic("max depth of %d exceeded", d.maxDepth)
+		}
+		d.depth++
+		t.XdrRecurse(d, field)
+		d.depth--
+	default:
+		d.decoder.Marshal(field, t)
+	}
+}
+
+// ValidateTransactionEnvelope validates the given transaction envelope
+// to make sure that it does not contain malicious arrays or nested
+// structures which are too deep
+func ValidateTransactionEnvelope(b64Envelope string, maxDepth int) error {
+	return validate(b64Envelope, &TransactionEnvelope{}, maxDepth)
+}
+
+// ValidateLedgerKey validates the given ledger key
+// to make sure that it does not contain malicious arrays or nested
+// structures which are too deep
+func ValidateLedgerKey(b64Key string, maxDepth int) error {
+	return validate(b64Key, &LedgerKey{}, maxDepth)
+}
+
+func validate(b64 string, val goxdr.XdrType, maxDepth int) (err error) {
+	d := &depthLimiter{
+		depth:    0,
+		maxDepth: maxDepth,
+		decoder: &goxdr.XdrIn{
+			In: base64.NewDecoder(base64.StdEncoding, strings.NewReader(b64)),
+		},
+	}
+
+	defer func() {
+		switch i := recover().(type) {
+		case nil:
+		case goxdr.XdrError:
+			err = i
+		default:
+			panic(i)
+		}
+	}()
+	val.XdrMarshal(d, "")
+	return nil
+}

gxdr/validator_test.go

@@ -0,0 +1,99 @@
+package gxdr
+
+import (
+	"encoding/base64"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	goxdr "github.com/xdrpp/goxdr/xdr"
+)
+
+func buildVec(depth int) SCVal {
+	if depth <= 0 {
+		symbol := SCSymbol("s")
+		return SCVal{
+			Type: SCV_SYMBOL,
+			_u:   &symbol,
+		}
+	}
+	vec := &SCVec{
+		buildVec(depth - 1),
+	}
+	return SCVal{Type: SCV_VEC, _u: &vec}
+}
+
+func buildMaliciousVec(t *testing.T) string {
+	vals := &SCVec{}
+	for i := 0; i < 0x0D; i++ {
+		symbol := SCSymbol("s")
+		*vals = append(*vals, SCVal{
+			Type: SCV_SYMBOL,
+			_u:   &symbol,
+		})
+	}
+	vec := SCVal{Type: SCV_VEC, _u: &vals}
+	raw := Dump(&vec)
+	// raw[8-11] represents the part of the xdr that holds the
+	// length of the vector
+	for i, b := range raw {
+		if b == 0x0D {
+			assert.Equal(t, 11, i)
+		}
+	}
+	// here we override the most significant byte in the vector length
+	// so that the vector length in the xdr is 0xFA00000D which
+	// is equal to 4194304013
+	raw[8] = 0xFA
+	return base64.StdEncoding.EncodeToString(raw)
+}
+
+func TestValidator(t *testing.T) {
+	shallowVec := buildVec(2)
+	deepVec := buildVec(100)
+	for _, testCase := range []struct {
+		name          string
+		input         string
+		maxDepth      int
+		val           goxdr.XdrType
+		expectedError string
+	}{
+		{
+			"invalid base 64 input",
+			"{}<>~!@$#",
+			500,
+			&LedgerEntry{},
+			"illegal base64 data at input byte 0",
+		},
+		{
+			"valid depth",
+			base64.StdEncoding.EncodeToString(Dump(&shallowVec)),
+			500,
+			&SCVal{},
+			"",
+		},
+		{
+			"invalid depth",
+			base64.StdEncoding.EncodeToString(Dump(&deepVec)),
+			50,
+			&SCVal{},
+			"max depth of 50 exceeded",
+		},
+		{
+			"malicious length",
+			buildMaliciousVec(t),
+			500,
+			&SCVal{},
+			"EOF",
+		},
+	} {
+		t.Run(testCase.name, func(t *testing.T) {
+			err := validate(testCase.input, testCase.val, testCase.maxDepth)
+			if testCase.expectedError == "" {
+				assert.NoError(t, err)
+				assert.Equal(t, testCase.input, base64.StdEncoding.EncodeToString(Dump(testCase.val)))
+			} else {
+				assert.EqualError(t, err, testCase.expectedError)
+			}
+		})
+	}
+}

services/horizon/internal/actions/submit_transaction.go

@@ -6,6 +6,7 @@ import (
 	"mime"
 	"net/http"
 
+	"github.com/stellar/go/gxdr"
 	"github.com/stellar/go/network"
 	"github.com/stellar/go/protocols/horizon"
 	hProblem "github.com/stellar/go/services/horizon/internal/render/problem"
@@ -37,6 +38,9 @@ type envelopeInfo struct {
 
 func extractEnvelopeInfo(raw string, passphrase string) (envelopeInfo, error) {
 	result := envelopeInfo{raw: raw}
+	if err := gxdr.ValidateTransactionEnvelope(raw, gxdr.DefaultMaxDepth); err != nil {
+		return result, err
+	}
 	err := xdr.SafeUnmarshalBase64(raw, &result.parsed)
 	if err != nil {
 		return result, err

services/horizon/internal/app.go

@@ -521,6 +521,7 @@ func (a *App) init() error {
 		SSEUpdateFrequency:       a.config.SSEUpdateFrequency,
 		StaleThreshold:           a.config.StaleThreshold,
 		ConnectionTimeout:        a.config.ConnectionTimeout,
+		MaxHTTPRequestSize:       a.config.MaxHTTPRequestSize,
 		NetworkPassphrase:        a.config.NetworkPassphrase,
 		MaxPathLength:            a.config.MaxPathLength,
 		MaxAssetsPerPathRequest:  a.config.MaxAssetsPerPathRequest,

services/horizon/internal/config.go

@@ -40,6 +40,8 @@ type Config struct {
 
 	SSEUpdateFrequency time.Duration
 	ConnectionTimeout  time.Duration
+	// MaxHTTPRequestSize is the maximum allowed request payload size
+	MaxHTTPRequestSize uint
 	RateQuota          *throttled.RateQuota
 	FriendbotURL       *url.URL
 	LogLevel           logrus.Level

services/horizon/internal/flags.go

@@ -12,6 +12,8 @@ import (
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/viper"
 
+	"github.com/stellar/throttled"
+
 	"github.com/stellar/go/ingest/ledgerbackend"
 	"github.com/stellar/go/network"
 	"github.com/stellar/go/services/horizon/internal/db2/schema"
@@ -20,7 +22,6 @@ import (
 	"github.com/stellar/go/support/db"
 	"github.com/stellar/go/support/errors"
 	"github.com/stellar/go/support/log"
-	"github.com/stellar/throttled"
 )
 
 const (
@@ -62,6 +63,8 @@ const (
 	StellarPubnet = "pubnet"
 	// StellarTestnet is a constant representing the Stellar test network
 	StellarTestnet = "testnet"
+
+	defaultMaxHTTPRequestSize = uint(200 * 1024)
 )
 
 // validateBothOrNeither ensures that both options are provided, if either is provided.
@@ -366,6 +369,13 @@ func Flags() (*Config, support.ConfigOptions) {
 			CustomSetValue: support.SetDuration,
 			Usage:          "defines the timeout of connection after which 504 response will be sent or stream will be closed, if Horizon is behind a load balancer with idle connection timeout, this should be set to a few seconds less that idle timeout, does not apply to POST /transactions",
 		},
+		&support.ConfigOption{
+			Name:        "max-http-request-size",
+			ConfigKey:   &config.MaxHTTPRequestSize,
+			OptType:     types.Uint,
+			FlagDefault: defaultMaxHTTPRequestSize,
+			Usage:       "sets the limit on the maximum allowed http request payload size, default is 200kb, to disable the limit check, set to 0, only do so if you acknowledge the implications of accepting unbounded http request payload sizes.",
+		},
 		&support.ConfigOption{
 			Name:        "per-hour-rate-limit",
 			ConfigKey:   &config.RateQuota,

services/horizon/internal/httpx/router.go

@@ -38,6 +38,7 @@ type RouterConfig struct {
 	SSEUpdateFrequency       time.Duration
 	StaleThreshold           uint
 	ConnectionTimeout        time.Duration
+	MaxHTTPRequestSize       uint
 	NetworkPassphrase        string
 	MaxPathLength            uint
 	MaxAssetsPerPathRequest  int
@@ -89,6 +90,11 @@ func (r *Router) addMiddleware(config *RouterConfig,
 	}))
 	r.Use(loggerMiddleware(serverMetrics))
 	r.Use(timeoutMiddleware(config.ConnectionTimeout))
+	if config.MaxHTTPRequestSize > 0 {
+		r.Use(func(handler http.Handler) http.Handler {
+			return http.MaxBytesHandler(handler, int64(config.MaxHTTPRequestSize))
+		})
+	}
 	r.Use(recoverMiddleware)
 	r.Use(chimiddleware.Compress(flate.DefaultCompression, "application/hal+json"))
 

services/horizon/internal/integration/txsub_test.go

@@ -3,9 +3,12 @@ package integration
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/stellar/go/clients/horizonclient"
 	"github.com/stellar/go/services/horizon/internal/test/integration"
 	"github.com/stellar/go/txnbuild"
-	"github.com/stretchr/testify/assert"
 )
 
 func TestTxSub(t *testing.T) {
@@ -50,3 +53,42 @@ func TestTxSub(t *testing.T) {
 		assert.Error(t, err)
 	})
 }
+
+func TestTxSubLimitsBodySize(t *testing.T) {
+	if integration.GetCoreMaxSupportedProtocol() < 20 {
+		t.Skip("This test run does not support less than Protocol 20")
+	}
+
+	itest := integration.NewTest(t, integration.Config{
+		ProtocolVersion:  20,
+		EnableSorobanRPC: true,
+		HorizonEnvironment: map[string]string{
+			"MAX_HTTP_REQUEST_SIZE": "1800",
+		},
+	})
+
+	// establish which account will be contract owner, and load it's current seq
+	sourceAccount, err := itest.Client().AccountDetail(horizonclient.AccountRequest{
+		AccountID: itest.Master().Address(),
+	})
+	require.NoError(t, err)
+
+	installContractOp := assembleInstallContractCodeOp(t, itest.Master().Address(), "soroban_sac_test.wasm")
+	preFlightOp, minFee := itest.PreflightHostFunctions(&sourceAccount, *installContractOp)
+	_, err = itest.SubmitOperationsWithFee(&sourceAccount, itest.Master(), minFee, &preFlightOp)
+	assert.EqualError(
+		t, err,
+		"horizon error: \"Transaction Malformed\" - check horizon.Error.Problem for more information",
+	)
+
+	sourceAccount, err = itest.Client().AccountDetail(horizonclient.AccountRequest{
+		AccountID: itest.Master().Address(),
+	})
+	require.NoError(t, err)
+
+	installContractOp = assembleInstallContractCodeOp(t, itest.Master().Address(), "soroban_add_u64.wasm")
+	preFlightOp, minFee = itest.PreflightHostFunctions(&sourceAccount, *installContractOp)
+	tx, err := itest.SubmitOperationsWithFee(&sourceAccount, itest.Master(), minFee, &preFlightOp)
+	require.NoError(t, err)
+	require.True(t, tx.Successful)
+}