From 1ebe1844d2add1c7c9fa8eae868371e8fc8eb250 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Thu, 20 Dec 2018 15:46:51 +0100 Subject: [PATCH] validate sign tx request's body Closes: #3176 --- PENDING.md | 8 +++++-- client/lcd/lcd_test.go | 11 +++++----- client/lcd/test_helpers.go | 10 ++++----- client/utils/rest.go | 2 +- x/auth/client/rest/sign.go | 45 +++++++++++++++++++++++--------------- 5 files changed, 43 insertions(+), 33 deletions(-) diff --git a/PENDING.md b/PENDING.md index 101a306508..07e8fc1352 100644 --- a/PENDING.md +++ b/PENDING.md @@ -17,6 +17,9 @@ BREAKING CHANGES * [\#3162](https://github.com/cosmos/cosmos-sdk/issues/3162) The `--gas` flag now takes `auto` instead of `simulate` in order to trigger a simulation of the tx before the actual execution. +* Gaia REST API + * [\#3176](https://github.com/cosmos/cosmos-sdk/issues/3176) `tx/sign` endpoint now expects `BaseReq` fields as nested object. + * SDK * [\#3064](https://github.com/cosmos/cosmos-sdk/issues/3064) Sanitize `sdk.Coin` denom. Coins denoms are now case insensitive, i.e. 100fooToken equals to 100FOOTOKEN. @@ -48,7 +51,8 @@ FEATURES IMPROVEMENTS -* Gaia REST API (`gaiacli advanced rest-server`) +* Gaia REST API + * [\#3176](https://github.com/cosmos/cosmos-sdk/issues/3176) Validate tx/sign endpoint POST body. * Gaia CLI (`gaiacli`) @@ -72,7 +76,7 @@ IMPROVEMENTS BUG FIXES -* Gaia REST API (`gaiacli advanced rest-server`) +* Gaia REST API * Gaia CLI (`gaiacli`) * \#3141 Fix the bug in GetAccount when `len(res) == 0` and `err == nil` diff --git a/client/lcd/lcd_test.go b/client/lcd/lcd_test.go index cff3bc70dc..78d8aa9f29 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" @@ -259,12 +260,10 @@ 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.NewBaseReq( + name1, pw, "", viper.GetString(client.FlagChainID), "", "", accnum, sequence, nil, false, false, + ), } json, err := cdc.MarshalJSON(payload) require.Nil(t, err) diff --git a/client/lcd/test_helpers.go b/client/lcd/test_helpers.go index e6e2d2fa40..0bdd519f68 100644 --- a/client/lcd/test_helpers.go +++ b/client/lcd/test_helpers.go @@ -645,12 +645,10 @@ 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.NewBaseReq( + name, password, "", chainID, "", "", accnum, sequence, nil, false, false, + ), } json, err := cdc.MarshalJSON(payload) require.Nil(t, err) diff --git a/client/utils/rest.go b/client/utils/rest.go index e7e0f33714..2cdecab64c 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, fmt.Sprintf("failed to decode JSON payload: %s", err)) return err } diff --git a/x/auth/client/rest/sign.go b/x/auth/client/rest/sign.go index 41e95e322f..a83e7be7d0 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"` + BaseReq utils.BaseReq `json:"base_req"` } // nolint: unparam @@ -30,21 +26,34 @@ 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 { - utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) - return - } - err = cdc.UnmarshalJSON(body, &m) - if err != nil { + if err := utils.ReadRESTReq(w, r, cdc, &m); err != nil { utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } - 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) + if !m.BaseReq.ValidateBasic(w) { + return + } - signedTx, err := txBldr.SignStdTx(m.LocalAccountName, m.Password, m.Tx, m.AppendSig) + // 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 + } + + txBldr := authtxb.NewTxBuilder( + utils.GetTxEncoder(cdc), + m.BaseReq.AccountNumber, + m.BaseReq.Sequence, + m.Tx.Fee.Gas, + 1.0, + false, + m.BaseReq.ChainID, + m.Tx.GetMemo(), + m.Tx.Fee.Amount) + + signedTx, err := txBldr.SignStdTx(m.BaseReq.Name, m.BaseReq.Password, m.Tx, m.AppendSig) if keyerror.IsErrKeyNotFound(err) { utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return