diff --git a/consensus/action/capability.go b/consensus/action/capability.go index 6c0afa0b2..28f391e27 100644 --- a/consensus/action/capability.go +++ b/consensus/action/capability.go @@ -254,6 +254,10 @@ func (c *consensusCapability) Simple(ctx context.Context, metadata capabilities. responseAndMetadata := capabilities.ResponseAndMetadata[*valuespb.Value]{} return &responseAndMetadata, caperrors.NewPublicSystemError(fmt.Errorf("failed to decode observation: %s", err), caperrors.InvalidArgument) } + if err := validateSimpleConsensusInput(input); err != nil { + responseAndMetadata := capabilities.ResponseAndMetadata[*valuespb.Value]{} + return &responseAndMetadata, caperrors.NewPublicSystemError(fmt.Errorf("invalid consensus input: %s", err), caperrors.InvalidArgument) + } lggr.Debugw("received simple consensus request", "metadata", metadata) @@ -652,3 +656,13 @@ func decodeObservationType(lggr logger.Logger, input *sdk.SimpleConsensusInputs) } return nil } + +func validateSimpleConsensusInput(input *sdk.SimpleConsensusInputs) error { + if valueInput, ok := input.GetObservation().(*sdk.SimpleConsensusInputs_Value); ok { + if err := oracle.ValidateAggregatableValue(valueInput.Value); err != nil { + return fmt.Errorf("invalid observation value: %w", err) + } + } + + return nil +} diff --git a/consensus/oracle/consensus_execution.go b/consensus/oracle/consensus_execution.go index 05fcc6d4a..038da4d05 100644 --- a/consensus/oracle/consensus_execution.go +++ b/consensus/oracle/consensus_execution.go @@ -27,10 +27,40 @@ var ( ErrDecimalExponentOutOfRange = errors.New("decimal exponent out of range") ) -// Comparing decimals rescales them to a common exponent, which costs time and memory -// proportional to the exponent gap, so a single observation with an extreme exponent -// could stall every node computing the median. -const maxDecimalExponent = 1000 +// MaxAggregatableExponent bounds the base-10 exponent of an aggregatable value. +// Comparing decimals with different exponents rescales them to a common exponent, +// and that cost grows with the exponent gap. Every in-tree producer reports +// exponent 0; the symmetric window leaves room for scaled values. +const MaxAggregatableExponent = 128 + +// ValidateAggregatableValue rejects decimal exponents that could make decimal +// comparisons allocate unbounded amounts of memory during aggregation. +func ValidateAggregatableValue(value *valuespb.Value) error { + if value == nil { + return nil + } + + switch v := value.Value.(type) { + case *valuespb.Value_DecimalValue: + if exponent := v.DecimalValue.GetExponent(); exponent > MaxAggregatableExponent || exponent < -MaxAggregatableExponent { + return fmt.Errorf("decimal exponent out of range: got %d, allowed [%d, %d]", exponent, -MaxAggregatableExponent, MaxAggregatableExponent) + } + case *valuespb.Value_MapValue: + for _, field := range v.MapValue.GetFields() { + if err := ValidateAggregatableValue(field); err != nil { + return err + } + } + case *valuespb.Value_ListValue: + for _, field := range v.ListValue.GetFields() { + if err := ValidateAggregatableValue(field); err != nil { + return err + } + } + } + + return nil +} // Constants for type names used in aggregation logic. var ( @@ -258,7 +288,7 @@ func handleMedianAggregation( filtered, func(val *valuespb.Value) (decimal.Decimal, error) { var d decimal.Decimal - if exp := val.GetDecimalValue().GetExponent(); exp > maxDecimalExponent || exp < -maxDecimalExponent { + if exp := val.GetDecimalValue().GetExponent(); exp > MaxAggregatableExponent || exp < -MaxAggregatableExponent { return d, fmt.Errorf("%w: %d", ErrDecimalExponentOutOfRange, exp) } v, err := values.FromProto(val) diff --git a/consensus/oracle/plugin/plugin_outcome.go b/consensus/oracle/plugin/plugin_outcome.go index 028256326..963640d50 100644 --- a/consensus/oracle/plugin/plugin_outcome.go +++ b/consensus/oracle/plugin/plugin_outcome.go @@ -67,6 +67,13 @@ func (r *reportingPlugin) ValidateObservation(ctx context.Context, outctx ocr3ty lggr.Warnw("observation missing input", "requestID", requestID) return fmt.Errorf("observation from observer %d is missing input for request %s", ao.Observer, requestID) } + + if valueInput, ok := reqObs.Input.GetObservation().(*sdk.SimpleConsensusInputs_Value); ok { + if err := oracle.ValidateAggregatableValue(valueInput.Value); err != nil { + lggr.Warnw("observation contains an aggregatable value outside the supported exponent range", "requestID", requestID, "error", err) + return fmt.Errorf("observation from observer %d has invalid value for request %s: %w", ao.Observer, requestID, err) + } + } } return nil diff --git a/consensus/oracle/plugin/plugin_outcome_test.go b/consensus/oracle/plugin/plugin_outcome_test.go index 750d46fec..786a1c336 100644 --- a/consensus/oracle/plugin/plugin_outcome_test.go +++ b/consensus/oracle/plugin/plugin_outcome_test.go @@ -4,9 +4,11 @@ import ( "context" "errors" "fmt" + "math" "testing" "time" + "github.com/shopspring/decimal" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" @@ -24,6 +26,7 @@ import ( "github.com/smartcontractkit/chainlink-protos/cre/go/sdk" "github.com/smartcontractkit/chainlink-protos/cre/go/values" + valuespb "github.com/smartcontractkit/chainlink-protos/cre/go/values/pb" "github.com/smartcontractkit/libocr/commontypes" "github.com/smartcontractkit/libocr/offchainreporting2plus/ocr3types" @@ -227,6 +230,31 @@ func Test_ValidateObservation_RequestIDConsistency(t *testing.T) { require.ErrorContains(t, err, "not in the query") } +func Test_ValidateObservation_RejectsExtremeDecimalExponent(t *testing.T) { + t.Parallel() + + lggr := logger.Test(t) + reportingPlugin, _ := createReportingPlugin(t, lggr, 2, 7, 5, defaultMaxLengthBytes) + + md := testMetaData() + reqID := md.RequestID() + qBytes, err := proto.Marshal(&oracletypes.Query{RequestIDs: []string{reqID}}) + require.NoError(t, err) + + ao := makeOutcomeTestObs(t, reqID, md, sdk.AggregationType_AGGREGATION_TYPE_MEDIAN, 0, false) + obs := &oracletypes.Observation{} + require.NoError(t, proto.Unmarshal(ao.Observation, obs)) + obs.Observations[reqID].Input.Observation = &sdk.SimpleConsensusInputs_Value{ + Value: valuespb.NewDecimalValue(decimal.New(1, math.MaxInt32)), + } + ao.Observation, err = proto.Marshal(obs) + require.NoError(t, err) + + err = reportingPlugin.ValidateObservation(t.Context(), ocr3types.OutcomeContext{SeqNr: 1}, qBytes, ao) + require.Error(t, err) + require.ErrorContains(t, err, "decimal exponent out of range") +} + // Test_Outcome_PlusOneErrors checks that when f+1 errors are received, Outcome() embeds the // per-field metadata string ("Consensus metadata: requestId=...") and the descriptor type // string ("Descriptor type: AGGREGATION_TYPE_MEDIAN").