From 266a9ddee39fd4ff6fde77fa2cba09a3c7a61b72 Mon Sep 17 00:00:00 2001
From: Alessio Treglia <alessio@tendermint.com>
Date: Thu, 20 Dec 2018 15:46:51 +0100
Subject: [PATCH] validate sign tx request's body

Closes: #3176
---
 PENDING.md                 |  1 +
 client/lcd/lcd_test.go     | 15 +++++++++------
 client/lcd/test_helpers.go | 14 ++++++++------
 client/utils/rest.go       |  2 +-
 x/auth/client/rest/sign.go | 28 +++++++++++++++-------------
 5 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/PENDING.md b/PENDING.md
index daab6b8d443..9386f6a0347 100644
--- a/PENDING.md
+++ b/PENDING.md
@@ -41,6 +41,7 @@ FEATURES
 IMPROVEMENTS
 
 * Gaia REST API (`gaiacli advanced rest-server`)
+  * [\#3176](https://github.com/cosmos/cosmos-sdk/issues/3176) Validate tx/sign endpoint POST body.
 
 * Gaia CLI  (`gaiacli`)
 
diff --git a/client/lcd/lcd_test.go b/client/lcd/lcd_test.go
index b6b7a771758..9338a69ef08 100644
--- a/client/lcd/lcd_test.go
+++ b/client/lcd/lcd_test.go
@@ -17,6 +17,7 @@ import (
 
 	client "github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/client/tx"
+	"github.com/cosmos/cosmos-sdk/client/utils"
 	"github.com/cosmos/cosmos-sdk/cmd/gaia/app"
 
 	"github.com/cosmos/cosmos-sdk/crypto/keys/mintkey"
@@ -246,12 +247,14 @@ func TestCoinSendGenerateSignAndBroadcast(t *testing.T) {
 	var signedMsg auth.StdTx
 
 	payload := authrest.SignBody{
-		Tx:               msg,
-		LocalAccountName: name1,
-		Password:         pw,
-		ChainID:          viper.GetString(client.FlagChainID),
-		AccountNumber:    accnum,
-		Sequence:         sequence,
+		Tx: msg,
+		BaseReq: utils.BaseReq{
+			Name:          name1,
+			Password:      pw,
+			ChainID:       viper.GetString(client.FlagChainID),
+			AccountNumber: accnum,
+			Sequence:      sequence,
+		},
 	}
 	json, err := cdc.MarshalJSON(payload)
 	require.Nil(t, err)
diff --git a/client/lcd/test_helpers.go b/client/lcd/test_helpers.go
index 850b4b95703..bc3ad20aa31 100644
--- a/client/lcd/test_helpers.go
+++ b/client/lcd/test_helpers.go
@@ -643,12 +643,14 @@ func getAccount(t *testing.T, port string, addr sdk.AccAddress) auth.Account {
 func doSign(t *testing.T, port, name, password, chainID string, accnum, sequence uint64, msg auth.StdTx) auth.StdTx {
 	var signedMsg auth.StdTx
 	payload := authrest.SignBody{
-		Tx:               msg,
-		LocalAccountName: name,
-		Password:         password,
-		ChainID:          chainID,
-		AccountNumber:    accnum,
-		Sequence:         sequence,
+		Tx: msg,
+		BaseReq: utils.BaseReq{
+			Name:          name,
+			Password:      password,
+			ChainID:       chainID,
+			AccountNumber: accnum,
+			Sequence:      sequence,
+		},
 	}
 	json, err := cdc.MarshalJSON(payload)
 	require.Nil(t, err)
diff --git a/client/utils/rest.go b/client/utils/rest.go
index e7e0f33714a..705b2618a69 100644
--- a/client/utils/rest.go
+++ b/client/utils/rest.go
@@ -189,7 +189,7 @@ func ReadRESTReq(w http.ResponseWriter, r *http.Request, cdc *codec.Codec, req i
 
 	err = cdc.UnmarshalJSON(body, req)
 	if err != nil {
-		WriteErrorResponse(w, http.StatusBadRequest, err.Error())
+		WriteErrorResponse(w, http.StatusBadRequest, "failed to decode JSON payload: "+err.Error())
 		return err
 	}
 
diff --git a/x/auth/client/rest/sign.go b/x/auth/client/rest/sign.go
index 41e95e322fc..29b842eb12f 100644
--- a/x/auth/client/rest/sign.go
+++ b/x/auth/client/rest/sign.go
@@ -1,26 +1,22 @@
 package rest
 
 import (
-	"io/ioutil"
 	"net/http"
 
 	"github.com/cosmos/cosmos-sdk/client/context"
 	"github.com/cosmos/cosmos-sdk/client/utils"
 	"github.com/cosmos/cosmos-sdk/codec"
 	"github.com/cosmos/cosmos-sdk/crypto/keys/keyerror"
+	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/auth"
 	authtxb "github.com/cosmos/cosmos-sdk/x/auth/client/txbuilder"
 )
 
 // SignBody defines the properties of a sign request's body.
 type SignBody struct {
-	Tx               auth.StdTx `json:"tx"`
-	LocalAccountName string     `json:"name"`
-	Password         string     `json:"password"`
-	ChainID          string     `json:"chain_id"`
-	AccountNumber    uint64     `json:"account_number"`
-	Sequence         uint64     `json:"sequence"`
-	AppendSig        bool       `json:"append_sig"`
+	Tx        auth.StdTx `json:"tx"`
+	AppendSig bool       `json:"append_sig"`
+	utils.BaseReq
 }
 
 // nolint: unparam
@@ -30,13 +26,19 @@ func SignTxRequestHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.Ha
 	return func(w http.ResponseWriter, r *http.Request) {
 		var m SignBody
 
-		body, err := ioutil.ReadAll(r.Body)
-		if err != nil {
+		if err := utils.ReadRESTReq(w, r, cdc, &m); err != nil {
 			utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
 			return
 		}
-		err = cdc.UnmarshalJSON(body, &m)
-		if err != nil {
+
+		// validate request
+		if !m.ValidateBasic(w) {
+			return
+		}
+
+		// validate tx
+		// discard error if it's CodeUnauthorized as the tx comes with no signatures
+		if err := m.Tx.ValidateBasic(); err != nil && err.Code() != sdk.CodeUnauthorized {
 			utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
 			return
 		}
@@ -44,7 +46,7 @@ func SignTxRequestHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.Ha
 		txBldr := authtxb.NewTxBuilder(utils.GetTxEncoder(cdc), m.AccountNumber,
 			m.Sequence, m.Tx.Fee.Gas, 1.0, false, m.ChainID, m.Tx.GetMemo(), m.Tx.Fee.Amount)
 
-		signedTx, err := txBldr.SignStdTx(m.LocalAccountName, m.Password, m.Tx, m.AppendSig)
+		signedTx, err := txBldr.SignStdTx(m.Name, m.Password, m.Tx, m.AppendSig)
 		if keyerror.IsErrKeyNotFound(err) {
 			utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
 			return