Skip to content

Commit 84d54ae

Browse files
committed
Update Not So Cosmos for Miss Erroe Handler
1 parent 0a764bb commit 84d54ae

File tree

2 files changed

+47
-1
lines changed

2 files changed

+47
-1
lines changed

not-so-smart-contracts/cosmos/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ Each _Not So Smart Cosmos_ includes a standard set of information:
2424
| [Broken bookkeeping](broken_bookkeeping) | Exploit mismatch between different modules' views on balances |
2525
| [Rounding errors](rounding_errors) | Bugs related to imprecision of finite precision arithmetic |
2626
| [Unregistered message handler](unregistered_msg_handler) | Broken functionality because of unregistered msg handler |
27-
27+
| [Missing error handler](missing_error_handler) | Missing error handling leads to successful execution of a transaction that should have failed |
2828
## Credits
2929

3030
These examples are developed and maintained by [Trail of Bits](https://www.trailofbits.com/).
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# Missing error handler
2+
3+
The idiomatic way of handling errors in `Go` is to compare the returned error to nil. This way of checking for errors gives the programmer a lot of control. However, when error handling is ignored it can also lead to numerous problems. The impact of this is most obvious in method calls in the `bankKeeper` module, which even causes some accounts with insufficient balances to perform `SendCoin` operations normally without triggering a transaction failure.
4+
5+
6+
## Example
7+
In the following code, `k.bankKeeper.SendCoins(ctx, lender, borrower, amount)` does not have any return values being used, including `err`. This results in `SendCoin` not being able to prevent the transaction from executing even if there is an `error` due to insufficient balance in `SendCoin`.
8+
```golang
9+
func (k msgServer) ApproveLoan(goCtx context.Context, msg *types.MsgApproveLoan) (*types.MsgApproveLoanResponse, error) {
10+
ctx := sdk.UnwrapSDKContext(goCtx)
11+
12+
loan, found := k.GetLoans(ctx, msg.Id)
13+
if !found {
14+
return nil, sdkerrors.Wrapf(sdkerrors.ErrKeyNotFound, "key %d doesn't exist", msg.Id)
15+
}
16+
17+
// TODO: for some reason the error doesn't get printed to the terminal
18+
if loan.State != "requested" {
19+
return nil, sdkerrors.Wrapf(types.ErrWrongLoanState, "%v", loan.State)
20+
}
21+
22+
lender, _ := sdk.AccAddressFromBech32(msg.Creator)
23+
borrower, _ := sdk.AccAddressFromBech32(loan.Borrower)
24+
amount, err := sdk.ParseCoinsNormalized(loan.Amount)
25+
if err != nil {
26+
return nil, sdkerrors.Wrap(types.ErrWrongLoanState, "Cannot parse coins in loan amount")
27+
}
28+
29+
k.bankKeeper.SendCoins(ctx, lender, borrower, amount)
30+
31+
loan.Lender = msg.Creator
32+
loan.State = "approved"
33+
34+
k.SetLoans(ctx, loan)
35+
36+
return &types.MsgApproveLoanResponse{}, nil
37+
}
38+
```
39+
## Mitigations
40+
41+
- Implement the error handling process instead of missing it
42+
43+
## External examples
44+
- [ignite's tutorials](https://github.com/ignite/cli/issues/2828).
45+
- [JackalLabs](https://github.com/JackalLabs/canine-chain/issues/8).
46+
- [OllO](https://github.com/OllO-Station/ollo/issues/20)

0 commit comments

Comments
 (0)