LaconicNetwork/cosmos-sdk
7
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

R4R: validate sign tx request's body (#3179)

Browse Source 
* validate sign tx request's body

Closes: #3176

* Introduce CodeNoSignatures

* Update swagger.yaml

* Fix tests

* Update x/auth/client/rest/sign.go

Co-Authored-By: alessio <quadrispro@ubuntu.com>
This commit is contained in:
Jack Zampolin
2019-01-03 08:30:04 -08:00
committed by GitHub
co-authored by alessio
parent 7ff29246df 5448824efb
commit d1824ad704
10 changed files with 56 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
PENDING.md
+8 -2
View File
@@ -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
* [stake] \#2513 Validator power type from Dec -> Int
* [\#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.
@@ -44,13 +47,16 @@ FEATURES
* SDK
* \#2996 Update the `AccountKeeper` to contain params used in the context of
the ante handler.
* [\#3179](https://github.com/cosmos/cosmos-sdk/pull/3179) New CodeNoSignatures error code.
* Tendermint
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`)
@@ -74,7 +80,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`
client/lcd/lcd_test.go
+5 -6
View File
@@ -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)
client/lcd/swagger-ui/swagger.yaml
Vendored
+2 -12
View File
@@ -286,20 +286,10 @@ paths:
schema:
type: object
properties:
base_req:
$ref: "#/definitions/BaseReq"
tx:
$ref: "#/definitions/StdTx"
name:
type: string
password:
type: string
chain_id:
type: string
account_number:
type: string
example: "0"
sequence:
type: string
example: "0"
append_sig:
type: boolean
example: true
client/lcd/test_helpers.go
+4 -6
View File
@@ -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)
client/utils/rest.go
+1 -1
View File
@@ -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
}
types/errors.go
+6
View File
@@ -45,6 +45,7 @@ const (
CodeInsufficientFee CodeType = 14
CodeTooManySignatures CodeType = 15
CodeGasOverflow CodeType = 16
CodeNoSignatures CodeType = 17
// CodespaceRoot is a codespace for error codes in this file only.
// Notice that 0 is an "unset" codespace, which can be overridden with
@@ -90,6 +91,8 @@ func CodeToDefaultMsg(code CodeType) string {
return "insufficient fee"
case CodeTooManySignatures:
return "maximum numer of signatures exceeded"
case CodeNoSignatures:
return "no signatures supplied"
default:
return unknownCodeMsg(code)
}
@@ -145,6 +148,9 @@ func ErrInsufficientFee(msg string) Error {
func ErrTooManySignatures(msg string) Error {
return newErrorWithRootCodespace(CodeTooManySignatures, msg)
}
func ErrNoSignatures(msg string) Error {
return newErrorWithRootCodespace(CodeNoSignatures, msg)
}
func ErrGasOverflow(msg string) Error {
return newErrorWithRootCodespace(CodeGasOverflow, msg)
}
x/auth/ante_test.go
+1 -1
View File
@@ -71,7 +71,7 @@ func TestAnteHandlerSigErrors(t *testing.T) {
require.Equal(t, expectedSigners, stdTx.GetSigners())
// Check no signatures fails
checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeUnauthorized)
checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeNoSignatures)
// test num sigs dont match GetSigners
privs, accNums, seqs = []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0}
x/auth/client/rest/sign.go
+27 -18
View File
@@ -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 CodeNoSignatures as the tx comes with no signatures
if err := m.Tx.ValidateBasic(); err != nil && err.Code() != sdk.CodeNoSignatures {
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
x/auth/stdtx.go
+1 -1
View File
@@ -50,7 +50,7 @@ func (tx StdTx) ValidateBasic() sdk.Error {
return sdk.ErrInsufficientFee(fmt.Sprintf("invalid fee %s amount provided", tx.Fee.Amount))
}
if len(stdSigs) == 0 {
return sdk.ErrUnauthorized("no signers")
return sdk.ErrNoSignatures("no signers")
}
if len(stdSigs) != len(tx.GetSigners()) {
return sdk.ErrUnauthorized("wrong number of signers")
x/auth/stdtx_test.go
+1 -1
View File
@@ -91,7 +91,7 @@ func TestTxValidateBasic(t *testing.T) {
err = tx.ValidateBasic()
require.Error(t, err)
require.Equal(t, sdk.CodeUnauthorized, err.Result().Code)
require.Equal(t, sdk.CodeNoSignatures, err.Result().Code)
// require to fail validation when signatures do not match expected signers
privs, accNums, seqs = []crypto.PrivKey{priv1}, []uint64{0, 1}, []uint64{0, 0}