Merge branch 'develop' into query-pending-nonces-by-chainID

.github/labeler.yml

@@ -4,3 +4,6 @@ breaking:proto:
 breaking:cli:
   - "x/*/client/cli/*.go"
   - "cmd/**/*.go"
+
+ci:
+  - ".github/**"

.github/workflows/sast-linters.yml

@@ -33,10 +33,33 @@ jobs:
       #   uses:  ./.github/actions/install-dependencies
 
       - name: Run Gosec Security Scanner
-        run: |
-          export PATH=$PATH:$(go env GOPATH)/bin
-          go install github.com/securego/gosec/v2/cmd/gosec@latest
-          gosec ./...
+        uses: securego/gosec@master
+        with:
+          args: ./...
+
+  gosec-cosmos:
+    runs-on: ubuntu-latest
+    env:
+      GO111MODULE: on
+    steps:
+      - name: Checkout Source
+        uses: actions/checkout@v2
+        with:
+          fetch-depth: 0
+
+      - name: Set up Go
+        uses: actions/setup-go@v3
+        with:
+          go-version: '1.20'
+
+      # - name: Install Pipeline Dependencies
+      #   uses:  ./.github/actions/install-dependencies
+
+      - name: Run Cosmos Gosec Security Scanner
+        uses: cosmos/gosec@master
+        with:
+          args: './... -include=G701,G703,G704' # Disabled G702 as it doesn't seem to be relevant 2023-09-14
+
 
   git-guardian:
     runs-on: ubuntu-latest
@@ -68,18 +91,18 @@ jobs:
         with:
           fetch-depth: 0
 
-      - name: Install Pipeline Dependencies
-        uses:  ./.github/actions/install-dependencies
+      # - name: Install Pipeline Dependencies
+      #   uses:  ./.github/actions/install-dependencies
 
       - name: Set up Go
         uses: actions/setup-go@v3
         with:
-          go-version: '1.19'
+          go-version: '1.20'
 
       - name: Run golangci-lint
         uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.50
+          version: v1.54
           skip-cache: true
           args: --timeout=15m
 
@@ -137,8 +160,11 @@ jobs:
 
             Be very careful about using `#nosec` in code. It can be a quick way to suppress security warnings and move forward with development, it should be employed with caution. Suppressing warnings with #nosec can hide potentially serious vulnerabilities. Only use #nosec when you're absolutely certain that the security issue is either a false positive or has been mitigated in another way.
 
+            Only suppress a single rule (or a specific set of rules) within a section of code, while continuing to scan for other problems. To do this, you can list the rule(s) to be suppressed within the #nosec annotation, e.g: /* #nosec G401 */ or //#nosec G201 G202 G203
+            Broad `#nosec` annotations should be avoided, as they can hide other vulnerabilities. **The CI will block you from merging this PR until you remove `#nosec` annotations that do not target specific rules**.
+
             Pay extra attention to the way `#nosec` is being used in the files listed above. 
-      
+
       - name: Add Label
         uses: actions/github-script@v6
         if: env.nosec_detected == 1
@@ -150,3 +176,11 @@ jobs:
               repo: context.repo.repo,
               labels: ["nosec"]
             })
+
+      - name: Check for '#nosec' without a specific rule
+        run: |
+          DIFF=$(git diff ${{ github.event.pull_request.base.sha }})
+          echo "$DIFF" | grep -P '#nosec(?!(\sG\d{3}))(?![^\s\t])([\s\t]*|$)' && echo "nosec without specified rule found!" && exit 1 || exit 0
+
+
+          

x/crosschain/keeper/evm_hooks.go

@@ -4,10 +4,12 @@ import (
 	"bytes"
 	"encoding/hex"
 	"fmt"
+	"math/big"
 	"strings"
 
 	errorsmod "cosmossdk.io/errors"
 	"cosmossdk.io/math"
+	"github.com/btcsuite/btcutil"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/ethereum/go-ethereum/accounts/abi"
 	"github.com/ethereum/go-ethereum/accounts/abi/bind"
@@ -256,10 +258,28 @@ func (k Keeper) ParseZRC20WithdrawalEvent(ctx sdk.Context, log ethtypes.Log) (*z
 		return nil, err
 	}
 
-	_, found := k.fungibleKeeper.GetForeignCoins(ctx, event.Raw.Address.Hex())
+	coin, found := k.fungibleKeeper.GetForeignCoins(ctx, event.Raw.Address.Hex())
 	if !found {
 		return nil, fmt.Errorf("ParseZRC20WithdrawalEvent: cannot find foreign coin with contract address %s", event.Raw.Address.Hex())
 	}
+	chainID := coin.ForeignChainId
+	if common.IsBitcoinChain(chainID) {
+		if event.Value.Cmp(big.NewInt(0)) <= 0 {
+			return nil, fmt.Errorf("ParseZRC20WithdrawalEvent: invalid amount %s", event.Value.String())
+		}
+		btcChainParams, err := common.GetBTCChainParams(chainID)
+		if err != nil {
+			return nil, err
+		}
+		addr, err := btcutil.DecodeAddress(string(event.To), btcChainParams)
+		if err != nil {
+			return nil, fmt.Errorf("ParseZRC20WithdrawalEvent: invalid address %s: %s", event.To, err)
+		}
+		_, ok := addr.(*btcutil.AddressWitnessPubKeyHash)
+		if !ok {
+			return nil, fmt.Errorf("ParseZRC20WithdrawalEvent: invalid address %s (not P2WPKH address)", event.To)
+		}
+	}
 	return event, nil
 }
 

x/fungible/keeper/evm.go

@@ -3,6 +3,7 @@ package keeper
 import (
 	"encoding/hex"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"math/big"
 	"strconv"
@@ -226,6 +227,43 @@ func (k Keeper) DepositZRC20AndCallContract(ctx sdk.Context,
 		"depositAndCall", context, zrc20Addr, amount, targetContract, message)
 }
 
+// QueryProtocolFlatFee returns the protocol flat fee associated with a given zrc20
+func (k Keeper) QueryProtocolFlatFee(ctx sdk.Context, contract common.Address) (*big.Int, error) {
+	zrc20ABI, err := zrc20.ZRC20MetaData.GetAbi()
+	if err != nil {
+		return nil, err
+	}
+	res, err := k.CallEVM(
+		ctx,
+		*zrc20ABI,
+		types.ModuleAddressEVM,
+		contract,
+		BigIntZero,
+		nil,
+		false,
+		false,
+		"PROTOCOL_FLAT_FEE",
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	unpacked, err := zrc20ABI.Unpack("PROTOCOL_FLAT_FEE", res.Ret)
+	if err != nil {
+		return nil, err
+	}
+	if len(unpacked) == 0 {
+		return nil, fmt.Errorf("expect 1 returned values, got %d", len(unpacked))
+	}
+
+	protocolGasFee, ok := unpacked[0].(*big.Int)
+	if !ok {
+		return nil, errors.New("can't read returned value as big.Int")
+	}
+
+	return protocolGasFee, nil
+}
+
 // QueryZRC20Data returns the data of a deployed ZRC20 contract
 func (k Keeper) QueryZRC20Data(
 	ctx sdk.Context,

x/fungible/keeper/msg_server_update_zrc20_withdraw_fee.go

@@ -2,7 +2,8 @@ package keeper
 
 import (
 	"context"
-	"math/big"
+
+	cosmoserrors "cosmossdk.io/errors"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
@@ -14,55 +15,50 @@ import (
 
 func (k Keeper) UpdateZRC20WithdrawFee(goCtx context.Context, msg *types.MsgUpdateZRC20WithdrawFee) (*types.MsgUpdateZRC20WithdrawFeeResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
+
+	// check signer permission
 	if msg.Creator != k.observerKeeper.GetParams(ctx).GetAdminPolicyAccount(zetaObserverTypes.Policy_Type_deploy_fungible_coin) {
-		return nil, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "Deploy can only be executed by the correct policy account")
+		return nil, cosmoserrors.Wrap(sdkerrors.ErrUnauthorized, "deploy can only be executed by the correct policy account")
 	}
+
+	// check the zrc20 exists
 	zrc20Addr := ethcommon.HexToAddress(msg.Zrc20Address)
 	if zrc20Addr == (ethcommon.Address{}) {
-		return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid zrc20 contract address (%s)", msg.Zrc20Address)
-	}
-
-	// update contracts
-	zrc20ABI, err := zrc20.ZRC20MetaData.GetAbi()
-	if err != nil {
-		return nil, sdkerrors.Wrapf(types.ErrABIGet, "failed to get zrc20 abi")
-	}
-
-	foreignCoins := k.GetAllForeignCoins(ctx)
-	found := false
-	var coin types.ForeignCoins
-	for _, fcoin := range foreignCoins {
-		coinZRC20Addr := ethcommon.HexToAddress(fcoin.Zrc20ContractAddress)
-		if coinZRC20Addr == (ethcommon.Address{}) {
-			k.Logger(ctx).Error("invalid zrc20 contract address", "address", fcoin.Zrc20ContractAddress)
-			continue
-		}
-		if coinZRC20Addr == zrc20Addr {
-			coin = fcoin
-			found = true
-			break
-		}
+		return nil, cosmoserrors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid zrc20 contract address (%s)", msg.Zrc20Address)
 	}
-
+	coin, found := k.GetForeignCoins(ctx, msg.Zrc20Address)
 	if !found {
-		return nil, sdkerrors.Wrapf(types.ErrInvalidAddress, "no foreign coin match requested zrc20 address (%s)", msg.Zrc20Address)
+		return nil, cosmoserrors.Wrapf(types.ErrForeignCoinNotFound, "no foreign coin match requested zrc20 address (%s)", msg.Zrc20Address)
 	}
 
-	res, err := k.CallEVM(ctx, *zrc20ABI, types.ModuleAddressEVM, zrc20Addr, BigIntZero, nil, false, false, "PROTOCOL_FLAT_FEE")
+	// get the previous fee
+	oldWithdrawFee, err := k.QueryProtocolFlatFee(ctx, zrc20Addr)
 	if err != nil {
-		return nil, sdkerrors.Wrapf(types.ErrContractCall, "failed to call zrc20 contract PROTOCOL_FLAT_FEE method (%s)", err.Error())
-	}
-	unpacked, err := zrc20ABI.Unpack("PROTOCOL_FLAT_FEE", res.Ret)
-	if err != nil || len(unpacked) == 0 {
-		return nil, sdkerrors.Wrapf(types.ErrContractCall, "failed to unpack zrc20 contract PROTOCOL_FLAT_FEE method (%s)", err.Error())
+		return nil, cosmoserrors.Wrapf(types.ErrContractCall, "failed to query protocol flat fee (%s)", err.Error())
 	}
-	oldWithdrawFee, ok := unpacked[0].(*big.Int)
-	if !ok {
-		return nil, sdkerrors.Wrapf(types.ErrContractCall, "failed to interpret the returned unpacked zrc20 contract PROTOCOL_FLAT_FEE method; ret %x", res.Ret)
+
+	zrc20ABI, err := zrc20.ZRC20MetaData.GetAbi()
+	if err != nil {
+		return nil, cosmoserrors.Wrapf(types.ErrABIGet, "failed to get zrc20 abi")
 	}
 
+	// call the contract method to update the fee
 	tmpCtx, commit := ctx.CacheContext()
-	_, err = k.CallEVM(tmpCtx, *zrc20ABI, types.ModuleAddressEVM, zrc20Addr, BigIntZero, nil, true, false, "updateProtocolFlatFee", msg.NewWithdrawFee.BigInt())
+	_, err = k.CallEVM(
+		tmpCtx,
+		*zrc20ABI,
+		types.ModuleAddressEVM,
+		zrc20Addr,
+		BigIntZero,
+		nil,
+		true,
+		false,
+		"updateProtocolFlatFee",
+		msg.NewWithdrawFee.BigInt(),
+	)
+	if err != nil {
+		return nil, cosmoserrors.Wrapf(types.ErrContractCall, "failed to call zrc20 contract updateProtocolFlatFee method (%s)", err.Error())
+	}
 
 	err = ctx.EventManager().EmitTypedEvent(
 		&types.EventZRC20WithdrawFeeUpdated{
@@ -77,8 +73,9 @@ func (k Keeper) UpdateZRC20WithdrawFee(goCtx context.Context, msg *types.MsgUpda
 	)
 	if err != nil {
 		k.Logger(ctx).Error("failed to emit event", "error", err.Error())
-		return nil, sdkerrors.Wrapf(types.ErrEmitEvent, "failed to emit event (%s)", err.Error())
+		return nil, cosmoserrors.Wrapf(types.ErrEmitEvent, "failed to emit event (%s)", err.Error())
 	}
 	commit()
+
 	return &types.MsgUpdateZRC20WithdrawFeeResponse{}, nil
 }