Skip to content

Commit 98c125a

Browse files
hayarobihayarobi
committed
Fix wrong validation of RPC input argument
- fix unit tests
1 parent 8fc8088 commit 98c125a

File tree

3 files changed

+31
-27
lines changed

3 files changed

+31
-27
lines changed

‎rpc/grpcserver.go‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ var (
3838
)
3939

4040
var (
41-
ErrUninitAccessor = errors.New("accessor is not initialized")
41+
ErrUninitAccessor = errors.New("accessor is not initilized")
4242

4343
// ErrNotSupportedConsensus = errors.New("not supported by this consensus")
4444
)
@@ -305,7 +305,7 @@ func (rpc *AergoRPCService) getBlocks(ctx context.Context, in *types.ListParams)
305305
}
306306
}
307307
if in.Asc || in.Offset != 0 {
308-
err = errors.New("has unsupported param")
308+
err = errors.New("Has unsupported param")
309309
}
310310
} else {
311311
end := types.BlockNo(0)
@@ -564,7 +564,7 @@ func (rpc *AergoRPCService) GetTX(ctx context.Context, in *types.SingleBytes) (*
564564
if err := rpc.checkAuth(ctx, ReadBlockChain); err != nil {
565565
return nil, err
566566
}
567-
if len(in.Value) == 0 || len(in.Value) > maxTxHashLength {
567+
if in.Value == nil {
568568
return nil, status.Errorf(codes.InvalidArgument, "invalid input hash")
569569
}
570570
result, err := rpc.actorHelper.CallRequestDefaultTimeout(message.MemPoolSvc,
@@ -589,7 +589,7 @@ func (rpc *AergoRPCService) GetBlockTX(ctx context.Context, in *types.SingleByte
589589
if err := rpc.checkAuth(ctx, ReadBlockChain); err != nil {
590590
return nil, err
591591
}
592-
if len(in.Value) == 0 || len(in.Value) > maxTxHashLength {
592+
if in.Value == nil {
593593
return nil, status.Errorf(codes.InvalidArgument, "invalid input hash")
594594
}
595595
result, err := rpc.hub.RequestFuture(message.ChainSvc,
@@ -611,7 +611,7 @@ func (rpc *AergoRPCService) SendTX(ctx context.Context, tx *types.Tx) (*types.Co
611611
if err := rpc.checkAuth(ctx, WriteBlockChain); err != nil {
612612
return nil, err
613613
}
614-
if len(tx.Hash) == 0 || len(tx.Hash) > maxTxHashLength {
614+
if tx.Hash == nil {
615615
return nil, status.Errorf(codes.InvalidArgument, "invalid tx hash")
616616
}
617617
if tx.Body == nil {
@@ -686,7 +686,7 @@ func (rpc *AergoRPCService) CommitTX(ctx context.Context, in *types.TxList) (*ty
686686
return nil, status.Errorf(codes.InvalidArgument, "empty transaction")
687687
}
688688
for _, tx := range in.Txs {
689-
if tx.Body == nil || len(tx.Hash) == 0 || len(tx.Hash) > maxTxHashLength {
689+
if tx.Body == nil || len(tx.Hash) == 0 {
690690
return nil, status.Errorf(codes.InvalidArgument, "input tx is empty")
691691
}
692692
}
@@ -707,7 +707,7 @@ func (rpc *AergoRPCService) GetState(ctx context.Context, in *types.SingleBytes)
707707
if err := rpc.checkAuth(ctx, ReadBlockChain); err != nil {
708708
return nil, err
709709
}
710-
if len(in.Value) == 0 {
710+
if in.Value == nil {
711711
return nil, status.Errorf(codes.InvalidArgument, "input account is empty")
712712
}
713713

@@ -925,7 +925,7 @@ func (rpc *AergoRPCService) SignTX(ctx context.Context, in *types.Tx) (*types.Tx
925925
if err := rpc.checkAuth(ctx, WriteBlockChain); err != nil {
926926
return nil, err
927927
}
928-
if in.Body == nil || len(in.Hash) == 0 || len(in.Hash) > maxTxHashLength {
928+
if in.Body == nil || len(in.Hash) == 0 {
929929
return nil, status.Errorf(codes.InvalidArgument, "input tx is empty")
930930
}
931931

@@ -951,7 +951,7 @@ func (rpc *AergoRPCService) VerifyTX(ctx context.Context, in *types.Tx) (*types.
951951
return nil, err
952952
}
953953
//TODO : verify without account service
954-
if in.Body == nil || len(in.Hash) == 0 || len(in.Hash) > maxTxHashLength {
954+
if in.Body == nil || len(in.Hash) == 0 {
955955
return nil, status.Errorf(codes.InvalidArgument, "input tx is empty")
956956
}
957957

@@ -1120,7 +1120,7 @@ func (rpc *AergoRPCService) GetReceipt(ctx context.Context, in *types.SingleByte
11201120
if err := rpc.checkAuth(ctx, ReadBlockChain); err != nil {
11211121
return nil, err
11221122
}
1123-
if len(in.Value) == 0 || len(in.Value) > maxTxHashLength {
1123+
if len(in.Value) == 0 {
11241124
return nil, status.Errorf(codes.InvalidArgument, "input hash is empty")
11251125
}
11261126
result, err := rpc.hub.RequestFuture(message.ChainSvc,
@@ -1174,7 +1174,7 @@ func (rpc *AergoRPCService) QueryContract(ctx context.Context, in *types.Query)
11741174
if err := rpc.checkAuth(ctx, ReadBlockChain); err != nil {
11751175
return nil, err
11761176
}
1177-
if len(in.ContractAddress) == 0 || len(in.Queryinfo) == 0 {
1177+
if len(in.ContractAddress) == 0 {
11781178
return nil, status.Errorf(codes.InvalidArgument, "input contract info is empty")
11791179
}
11801180
result, err := rpc.hub.RequestFuture(message.ChainSvc,
@@ -1194,7 +1194,7 @@ func (rpc *AergoRPCService) QueryContractState(ctx context.Context, in *types.St
11941194
if err := rpc.checkAuth(ctx, ReadBlockChain); err != nil {
11951195
return nil, err
11961196
}
1197-
if len(in.ContractAddress) == 0 || len(in.Root) == 0 {
1197+
if len(in.ContractAddress) == 0 {
11981198
return nil, status.Errorf(codes.InvalidArgument, "input contract info is empty")
11991199
}
12001200
result, err := rpc.hub.RequestFuture(message.ChainSvc,
@@ -1273,9 +1273,6 @@ func (rpc *AergoRPCService) ListEvents(ctx context.Context, in *types.FilterInfo
12731273
if err := rpc.checkAuth(ctx, ReadBlockChain); err != nil {
12741274
return nil, err
12751275
}
1276-
if len(in.ContractAddress) == 0 || len(in.ContractAddress) > types.AddressLength {
1277-
return nil, status.Errorf(codes.InvalidArgument, "input contract info is empty")
1278-
}
12791276
result, err := rpc.hub.RequestFuture(message.ChainSvc,
12801277
&message.ListEvents{Filter: in}, defaultActorTimeout, "rpc.(*AergoRPCService).ListEvents").Result()
12811278
if err != nil {

‎rpc/grpcserver_test.go‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -72,17 +72,10 @@ func init() {
7272
}
7373

7474
func TestAergoRPCService_GetTX(t *testing.T) {
75-
ctrl := gomock.NewController(t)
76-
defer ctrl.Finish()
77-
78-
mockMsgHelper := messagemock.NewHelper(ctrl)
79-
mockActorHelper := p2pmock.NewMockActorService(ctrl)
80-
8175
dummyTxBody := types.TxBody{Account: dummyWalletAddress, Amount: new(big.Int).SetUint64(4332).Bytes(),
8276
Recipient: dummyWalletAddress2, Payload: dummyPayload}
8377
sampleTx := &types.Tx{Hash: dummyTxHash, Body: &dummyTxBody}
84-
mockActorHelper.EXPECT().CallRequestDefaultTimeout(message.MemPoolSvc, gomock.Any()).Return(message.MemPoolGetRsp{}, nil)
85-
mockMsgHelper.EXPECT().ExtractTxFromResponse(gomock.AssignableToTypeOf(message.MemPoolGetRsp{})).Return(sampleTx, nil)
78+
8679
type fields struct {
8780
hub *component.ComponentHub
8881
actorHelper p2pcommon.ActorService
@@ -99,14 +92,28 @@ func TestAergoRPCService_GetTX(t *testing.T) {
9992
want *types.Tx
10093
wantErr bool
10194
}{
102-
{name: "T00", args: args{ctx: mockCtx, in: &types.SingleBytes{Value: append(dummyTxHash, 'b', 'd')}}, fields: fields{hubStub, mockActorHelper, mockMsgHelper},
95+
{name: "TNormal", args: args{ctx: mockCtx, in: &types.SingleBytes{Value: dummyTxHash}},
96+
want: &types.Tx{Hash: dummyTxHash, Body: &dummyTxBody}, wantErr: false},
97+
// TODO the malformed hash is allowed until v2.8.x , but should return error at v2.9.0
98+
{name: "TMalformedHash", args: args{ctx: mockCtx, in: &types.SingleBytes{Value: append(dummyTxHash, 'b', 'd')}},
99+
want: &types.Tx{Hash: dummyTxHash, Body: &dummyTxBody}, wantErr: false},
100+
{name: "TEmptyHash", args: args{ctx: mockCtx, in: &types.SingleBytes{Value: []byte{}}},
103101
want: &types.Tx{Hash: dummyTxHash, Body: &dummyTxBody}, wantErr: false},
104-
// TODO: Add test cases.
102+
{name: "TNilHash", args: args{ctx: mockCtx, in: &types.SingleBytes{Value: nil}},
103+
want: nil, wantErr: true},
105104
}
106105
for _, tt := range tests {
107106
t.Run(tt.name, func(t *testing.T) {
107+
ctrl := gomock.NewController(t)
108+
defer ctrl.Finish()
109+
110+
mockMsgHelper := messagemock.NewHelper(ctrl)
111+
mockActorHelper := p2pmock.NewMockActorService(ctrl)
112+
mockActorHelper.EXPECT().CallRequestDefaultTimeout(message.MemPoolSvc, gomock.Any()).Return(message.MemPoolGetRsp{}, nil).AnyTimes()
113+
mockMsgHelper.EXPECT().ExtractTxFromResponse(gomock.AssignableToTypeOf(message.MemPoolGetRsp{})).Return(sampleTx, nil).AnyTimes()
114+
108115
rpc := &AergoRPCService{
109-
hub: tt.fields.hub, actorHelper: mockActorHelper, msgHelper: mockMsgHelper,
116+
hub: hubStub, actorHelper: mockActorHelper, msgHelper: mockMsgHelper,
110117
}
111118
got, err := rpc.GetTX(tt.args.ctx, tt.args.in)
112119
if (err != nil) != tt.wantErr {

‎types/state_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func TestToAccountID(t *testing.T) {
6565
{"zeros", Address(zeroBytes),
6666
ToAccountID(zeroBytes)},
6767
{"empty", ToAddress(emptyAddress), ToAccountID([]byte{})},
68-
{"nil", ToAddress(emptyAddress), ToAccountID([]byte{})},
68+
{"nil", ToAddress(emptyAddress), ToAccountID(nil)},
6969
}
7070
for _, tt := range tests {
7171
t.Run(tt.name, func(t *testing.T) {

0 commit comments

Comments
 (0)