From 256f832642bc81596c8bd53498672922aac175d3 Mon Sep 17 00:00:00 2001
From: Acha Bill <57879913+acha-bill@users.noreply.github.com>
Date: Mon, 29 Jun 2020 11:08:49 +0100
Subject: [PATCH] Validate web API requests (#508)

* Adjust inbox worker pool capacity to default

* Adjust inbox worker pool capacity to default (#505)

* validate api requests

* release cachedInputs

Co-authored-by: Angelo Capossele <angelocapossele@gmail.com>

* add payload size validation to sendPayload api

* Apply suggestions from code review

* validate transaction signatures

* refactor: move validation to packages

* update ValidateTransactionToAttach

* define errors

* Apply suggestions from code review

* fixes from codereview

Co-authored-by: jonastheis <mail@jonastheis.de>
Co-authored-by: Wolfgang Welz <welzwo@gmail.com>
Co-authored-by: Angelo Capossele <angelocapossele@gmail.com>
Co-authored-by: Luca Moser <moser.luca@gmail.com>
---
 .../valuetransfers/packages/tangle/tangle.go  | 27 +++++++++++++++++++
 .../packages/transaction/transaction.go       | 10 ++++---
 .../collectiveBeacon/payload/payload.go       | 22 ++++++++++++---
 packages/binary/messagelayer/payload/data.go  |  3 +++
 .../binary/messagelayer/payload/payload.go    | 13 +++++++++
 plugins/webapi/data/plugin.go                 |  8 +++++-
 .../webapi/drng/collectivebeacon/handler.go   |  3 +--
 plugins/webapi/message/sendPayload.go         |  4 +--
 .../webapi/value/sendtransaction/handler.go   |  5 ++++
 9 files changed, 82 insertions(+), 13 deletions(-)

diff --git a/dapps/valuetransfers/packages/tangle/tangle.go b/dapps/valuetransfers/packages/tangle/tangle.go
index 7c91dec1..15b192b9 100644
--- a/dapps/valuetransfers/packages/tangle/tangle.go
+++ b/dapps/valuetransfers/packages/tangle/tangle.go
@@ -21,6 +21,12 @@ import (
 	"github.com/iotaledger/goshimmer/packages/binary/storageprefix"
 )
 
+var (
+	// ErrTransactionDoesNotSpendAllFunds is returned if a transaction does not spend all of its inputs.
+	ErrTransactionDoesNotSpendAllFunds = errors.New("transaction does not spend all funds from inputs")
+	// ErrInvalidTransactionSignature is returned if the signature of a transaction is invalid.
+	ErrInvalidTransactionSignature     = errors.New("invalid transaction signatures")
+)
 // Tangle represents the value tangle that consists out of value payloads.
 // It is an independent ontology, that lives inside the tangle.
 type Tangle struct {
@@ -1577,6 +1583,27 @@ func (tangle *Tangle) getCachedOutputsFromTransactionInputs(tx *transaction.Tran
 	return
 }
 
+// ValidateTransactionToAttach checks that the given transaction spends all funds from its inputs and
+// that its the signature is valid.
+func (tangle *Tangle) ValidateTransactionToAttach(tx *transaction.Transaction) (err error) {
+	_, cachedInputs, consumedBalances, _, err := tangle.retrieveConsumedInputDetails(tx)
+	defer cachedInputs.Release()
+	if err != nil {
+		return
+	}
+	if !tangle.checkTransactionOutputs(consumedBalances, tx.Outputs()) {
+		err = ErrTransactionDoesNotSpendAllFunds
+		return
+	}
+
+	if !tx.SignaturesValid() {
+		err = ErrInvalidTransactionSignature
+		return
+	}
+	return
+}
+
+// retrieveConsumedInputDetails retrieves the details of the consumed inputs of a transaction.
 func (tangle *Tangle) retrieveConsumedInputDetails(tx *transaction.Transaction) (inputsSolid bool, cachedInputs CachedOutputs, consumedBalances map[balance.Color]int64, consumedBranches branchmanager.BranchIds, err error) {
 	cachedInputs = tangle.getCachedOutputsFromTransactionInputs(tx)
 	consumedBalances = make(map[balance.Color]int64)
diff --git a/dapps/valuetransfers/packages/transaction/transaction.go b/dapps/valuetransfers/packages/transaction/transaction.go
index 3b234bdf..760ded1d 100644
--- a/dapps/valuetransfers/packages/transaction/transaction.go
+++ b/dapps/valuetransfers/packages/transaction/transaction.go
@@ -15,6 +15,11 @@ import (
 	"github.com/iotaledger/goshimmer/dapps/valuetransfers/packages/address/signaturescheme"
 )
 
+var (
+	// ErrMaxDataPayloadSizeExceeded is returned if the data payload size is exceeded.
+	ErrMaxDataPayloadSizeExceeded = errors.New("maximum data payload size exceeded")
+)
+
 // region IMPLEMENT Transaction ////////////////////////////////////////////////////////////////////////////////////////////
 
 // Transaction represents a value transfer for IOTA. It consists out of a number of inputs, a number of outputs and their
@@ -311,7 +316,7 @@ func (transaction *Transaction) SetDataPayload(data []byte) error {
 	defer transaction.dataPayloadMutex.Unlock()
 
 	if len(data) > MaxDataPayloadSize {
-		return fmt.Errorf("maximum dataPayload size of %d bytes exceeded", MaxDataPayloadSize)
+		return fmt.Errorf("%w: %d", ErrMaxDataPayloadSizeExceeded, MaxDataPayloadSize)
 	}
 	transaction.dataPayload = data
 	return nil
@@ -382,8 +387,7 @@ func (transaction *Transaction) UnmarshalObjectStorageValue(bytes []byte) (consu
 		return
 	}
 	if dataPayloadSize > MaxDataPayloadSize {
-		err = fmt.Errorf("data payload size of %d bytes exceeds maximum limit of %d bytes",
-			dataPayloadSize, MaxDataPayloadSize)
+		err = fmt.Errorf("%w: %d", ErrMaxDataPayloadSizeExceeded, MaxDataPayloadSize)
 		return
 	}
 
diff --git a/packages/binary/drng/subtypes/collectiveBeacon/payload/payload.go b/packages/binary/drng/subtypes/collectiveBeacon/payload/payload.go
index 2c41c2c8..d53be6f1 100644
--- a/packages/binary/drng/subtypes/collectiveBeacon/payload/payload.go
+++ b/packages/binary/drng/subtypes/collectiveBeacon/payload/payload.go
@@ -1,6 +1,8 @@
 package payload
 
 import (
+	"errors"
+	"fmt"
 	"sync"
 
 	"github.com/iotaledger/hive.go/stringify"
@@ -11,6 +13,11 @@ import (
 	"github.com/iotaledger/hive.go/marshalutil"
 )
 
+var (
+	// ErrMaximumPayloadSizeExceeded is returned if the payload exceeds the maximum size.
+	ErrMaximumPayloadSizeExceeded = errors.New("maximum payload size exceeded")
+)
+
 // Payload is a collective beacon payload.
 type Payload struct {
 	header.Header
@@ -28,6 +35,9 @@ type Payload struct {
 	bytesMutex sync.RWMutex
 }
 
+// MaxCollectiveBeaconPayloadSize defines the maximum size of a collective beacon payload.
+const MaxCollectiveBeaconPayloadSize = 64 * 1024
+
 // New creates a new collective beacon payload.
 func New(instanceID uint32, round uint64, prevSignature, signature, dpk []byte) *Payload {
 	return &Payload{
@@ -41,11 +51,15 @@ func New(instanceID uint32, round uint64, prevSignature, signature, dpk []byte)
 
 // Parse is a wrapper for simplified unmarshaling in a byte stream using the marshalUtil package.
 func Parse(marshalUtil *marshalutil.MarshalUtil) (*Payload, error) {
-	if payload, err := marshalUtil.Parse(func(data []byte) (interface{}, int, error) { return FromBytes(data) }); err != nil {
-		return &Payload{}, err
-	} else {
-		return payload.(*Payload), nil
+	unmarshalledPayload, err := marshalUtil.Parse(func(data []byte) (interface{}, int, error) { return FromBytes(data) })
+	if err != nil {
+		return nil, err
+	}
+	_payload := unmarshalledPayload.(*Payload)
+	if len(_payload.bytes) > MaxCollectiveBeaconPayloadSize {
+		return nil, fmt.Errorf("%w: %d", ErrMaximumPayloadSizeExceeded, MaxCollectiveBeaconPayloadSize)
 	}
+	return _payload, nil
 }
 
 // FromBytes parses the marshaled version of a Payload into an object.
diff --git a/packages/binary/messagelayer/payload/data.go b/packages/binary/messagelayer/payload/data.go
index 9db9cc64..6092956c 100644
--- a/packages/binary/messagelayer/payload/data.go
+++ b/packages/binary/messagelayer/payload/data.go
@@ -14,6 +14,9 @@ type Data struct {
 	data        []byte
 }
 
+// MaxDataPayloadSize defines the maximum size of a data payload.
+const MaxDataPayloadSize = 64 * 1024
+
 // NewData creates new data payload.
 func NewData(data []byte) *Data {
 	return &Data{
diff --git a/packages/binary/messagelayer/payload/payload.go b/packages/binary/messagelayer/payload/payload.go
index f70f8757..a0282c32 100644
--- a/packages/binary/messagelayer/payload/payload.go
+++ b/packages/binary/messagelayer/payload/payload.go
@@ -1,9 +1,17 @@
 package payload
 
 import (
+	"errors"
+	"fmt"
+
 	"github.com/iotaledger/hive.go/marshalutil"
 )
 
+var (
+	// ErrMaximumPayloadSizeExceeded is returned if the payload exceeds the maximum size.
+	ErrMaximumPayloadSizeExceeded = errors.New("maximum payload size exceeded")
+)
+
 const (
 	// ObjectName defines the name of the data object.
 	ObjectName = "data"
@@ -45,6 +53,11 @@ func FromBytes(bytes []byte) (result Payload, consumedBytes int, err error) {
 		return
 	}
 
+	if payloadSize > MaxDataPayloadSize {
+		err = fmt.Errorf("%w: %d", ErrMaximumPayloadSizeExceeded, MaxDataPayloadSize)
+		return
+	}
+
 	marshalUtil.ReadSeek(marshalUtil.ReadOffset() - marshalutil.UINT32_SIZE*2)
 	payloadBytes, err := marshalUtil.ReadBytes(int(payloadSize) + 8)
 	if err != nil {
diff --git a/plugins/webapi/data/plugin.go b/plugins/webapi/data/plugin.go
index a13ecce6..605ad8d8 100644
--- a/plugins/webapi/data/plugin.go
+++ b/plugins/webapi/data/plugin.go
@@ -1,6 +1,7 @@
 package data
 
 import (
+	"fmt"
 	"net/http"
 	"sync"
 
@@ -44,7 +45,12 @@ func broadcastData(c echo.Context) error {
 		return c.JSON(http.StatusBadRequest, Response{Error: err.Error()})
 	}
 
-	//TODO: to check max payload size allowed, if exceeding return an error
+	dataPayload := payload.NewData(request.Data)
+	if len(dataPayload.Bytes()) > payload.MaxDataPayloadSize {
+		err := fmt.Errorf("%w: %d", payload.ErrMaximumPayloadSizeExceeded, payload.MaxDataPayloadSize)
+		return c.JSON(http.StatusBadRequest, Response{Error: err.Error()})
+	}
+
 	msg, err := issuer.IssuePayload(payload.NewData(request.Data))
 	if err != nil {
 		return c.JSON(http.StatusBadRequest, Response{Error: err.Error()})
diff --git a/plugins/webapi/drng/collectivebeacon/handler.go b/plugins/webapi/drng/collectivebeacon/handler.go
index 591dc482..3c21953c 100644
--- a/plugins/webapi/drng/collectivebeacon/handler.go
+++ b/plugins/webapi/drng/collectivebeacon/handler.go
@@ -18,11 +18,10 @@ func Handler(c echo.Context) error {
 		return c.JSON(http.StatusBadRequest, Response{Error: err.Error()})
 	}
 
-	//TODO: to check max payload size allowed, if exceeding return an error
 	marshalUtil := marshalutil.New(request.Payload)
 	parsedPayload, err := payload.Parse(marshalUtil)
 	if err != nil {
-		return c.JSON(http.StatusBadRequest, Response{Error: "not a valid Collective Beacon payload"})
+		return c.JSON(http.StatusBadRequest, Response{Error: err.Error()})
 	}
 
 	msg, err := issuer.IssuePayload(parsedPayload)
diff --git a/plugins/webapi/message/sendPayload.go b/plugins/webapi/message/sendPayload.go
index 159ffe16..cffc572b 100644
--- a/plugins/webapi/message/sendPayload.go
+++ b/plugins/webapi/message/sendPayload.go
@@ -17,11 +17,9 @@ func sendPayload(c echo.Context) error {
 		return c.JSON(http.StatusBadRequest, MsgResponse{Error: err.Error()})
 	}
 
-	//TODO: to check max payload size allowed, if exceeding return an error
-
 	parsedPayload, _, err := payload.FromBytes(request.Payload)
 	if err != nil {
-		return c.JSON(http.StatusBadRequest, MsgResponse{Error: "not a valid payload"})
+		return c.JSON(http.StatusBadRequest, MsgResponse{Error: err.Error()})
 	}
 
 	msg, err := issuer.IssuePayload(parsedPayload)
diff --git a/plugins/webapi/value/sendtransaction/handler.go b/plugins/webapi/value/sendtransaction/handler.go
index 8a655715..dda59ab8 100644
--- a/plugins/webapi/value/sendtransaction/handler.go
+++ b/plugins/webapi/value/sendtransaction/handler.go
@@ -22,6 +22,11 @@ func Handler(c echo.Context) error {
 		return c.JSON(http.StatusBadRequest, Response{Error: err.Error()})
 	}
 
+	err = valuetransfers.Tangle().ValidateTransactionToAttach(tx)
+	if err != nil {
+		return c.JSON(http.StatusBadRequest, Response{Error: err.Error()})
+	}
+
 	// Prepare value payload and send the message to tangle
 	payload := valuetransfers.ValueObjectFactory().IssueTransaction(tx)
 	_, err = issuer.IssuePayload(payload)
-- 
GitLab