Skip to content

Commit

Permalink
validate sign tx request's body
Browse files Browse the repository at this point in the history
Closes: #3176
  • Loading branch information
Alessio Treglia committed Dec 27, 2018
1 parent 0d63c92 commit 1ebe184
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 30 deletions.
8 changes: 6 additions & 2 deletions PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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`)

Expand All @@ -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`
Expand Down
11 changes: 5 additions & 6 deletions client/lcd/lcd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 4 additions & 6 deletions client/lcd/test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion client/utils/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
39 changes: 24 additions & 15 deletions x/auth/client/rest/sign.go
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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 {
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 {

if !m.BaseReq.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
}

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)
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.LocalAccountName, m.Password, m.Tx, m.AppendSig)
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
Expand Down

0 comments on commit 1ebe184

Please sign in to comment.