stafiprotocol / security

0 stars 1 forks source link

This is my 4th attempt to find bugs, well it's impossible to find, I can't find any vulnerabilities. #34

Closed DimaKovalenko17 closed 1 year ago

DimaKovalenko17 commented 1 year ago

This is my 4th attempt to find bugs, well it's impossible to find, I can't find any vulnerabilities.

Potential for Front-Running: The stake and unstake functions could potentially be susceptible to front-running attacks, where an attacker sees a transaction in the pending pool and issues their own transaction with a higher gas price to get it processed first.

Reentrancy: The code does not seem to have reentrancy guards in place. If a function calls an external contract (like IStakePool(_poolAddress).withdrawForStaker(...) ) and then continues execution, the external contract could call back into the original contract and change state before the first function finishes. This can lead to unexpected behavior.

Gas Usage: The newEra() function loops over potentially large arrays ( poolList and validators ). This could cause the function to exceed the block gas limit if these arrays are too large, making the function unable to complete its execution.

The code does not have a function to update the protocolFeeCommission, rateChangeLimit, unbondingDuration and other such parameters. This could be a problem if you want to update these values in the future.

// ------ delegation balancer

function redelegate(
    address _poolAddress,
    uint256 _srcValidatorId,
    uint256 _dstValidatorId,
    uint256 _amount
) external onlyDelegationBalancer {
    require(validatorIdsOf[_poolAddress].contains(_srcValidatorId), "val not exist");
    require(_srcValidatorId != _dstValidatorId, "val duplicate");
    require(_amount > 0, "amount zero");

    if (!validatorIdsOf[_poolAddress].contains(_dstValidatorId)) {
        validatorIdsOf[_poolAddress].add(_dstValidatorId);
    }

    IStakePool(_poolAddress).redelegate(_srcValidatorId, _dstValidatorId, _amount);

    if (IStakePool(_poolAddress).getTotalStakeOnValidator(_srcValidatorId) == 0) {
        validatorIdsOf[_poolAddress].remove(_srcValidatorId);
    }
}

// ----- staker operation

function stake(uint256 _stakeAmount) external {
    stakeWithPool(bondedPools.at(0), _stakeAmount);
}

function unstake(uint256 _rTokenAmount) external {
    unstakeWithPool(bondedPools.at(0), _rTokenAmount);
}

function withdraw() external {
    withdrawWithPool(bondedPools.at(0));
}

function stakeWithPool(address _poolAddress, uint256 _stakeAmount) public {
    require(_stakeAmount >= minStakeAmount, "amount not enough");
    require(bondedPools.contains(_poolAddress), "pool not exist");

    uint256 rTokenAmount = _stakeAmount.mul(1e18).div(rate);

    // update pool
    PoolInfo storage poolInfo = poolInfoOf[_poolAddress];
    poolInfo.bond = poolInfo.bond.add(_stakeAmount);
    poolInfo.active = poolInfo.active.add(_stakeAmount);

    // transfer erc20 token
    IERC20(erc20TokenAddress).safeTransferFrom(msg.sender, _poolAddress, _stakeAmount);

    // mint rtoken
    totalRTokenSupply = totalRTokenSupply.add(rTokenAmount);
    IERC20MintBurn(rTokenAddress).mint(msg.sender, rTokenAmount);

    emit Stake(msg.sender, _poolAddress, _stakeAmount, rTokenAmount);
}

function unstakeWithPool(address _poolAddress, uint256 _rTokenAmount) public {
    require(_rTokenAmount > 0, "rtoken amount zero");
    require(bondedPools.contains(_poolAddress), "pool not exist");
    require(unstakesOfUser[msg.sender].length() <= UNBOND_TIMES_LIMIT, "unstake times limit");

    uint256 unstakeFee = _rTokenAmount.mul(unstakeFeeCommission).div(1e18);
    uint256 leftRTokenAmount = _rTokenAmount.sub(unstakeFee);
    uint256 tokenAmount = leftRTokenAmount.mul(rate).div(1e18);

    // update pool
    PoolInfo storage poolInfo = poolInfoOf[_poolAddress];
    poolInfo.unbond = poolInfo.unbond.add(tokenAmount);
    poolInfo.active = poolInfo.active.sub(tokenAmount);

    // burn rtoken
    IERC20MintBurn(rTokenAddress).burnFrom(msg.sender, leftRTokenAmount);
    totalRTokenSupply = totalRTokenSupply.sub(leftRTokenAmount);

    if (unstakeFee > 0) {
        // protocol fee
        totalProtocolFee = totalProtocolFee.add(unstakeFee);
        IERC20(rTokenAddress).safeTransferFrom(msg.sender, address(this), unstakeFee);
    }

    // unstake info
    uint256 willUseUnstakeIndex = nextUnstakeIndex;
    nextUnstakeIndex = willUseUnstakeIndex.add(1);

    unstakeAtIndex[willUseUnstakeIndex] = UnstakeInfo({
        era: currentEra(),
        pool: _poolAddress,
        receiver: msg.sender,
        amount: tokenAmount
    });
    unstakesOfUser[msg.sender].add(willUseUnstakeIndex);

    emit Unstake(msg.sender, _poolAddress, tokenAmount, _rTokenAmount, leftRTokenAmount, willUseUnstakeIndex);
}

function withdrawWithPool(address _poolAddress) public {
    uint256 totalWithdrawAmount;
    uint256 length = unstakesOfUser[msg.sender].length();
    uint256[] memory unstakeIndexList = new uint256[](length);
    int256[] memory emitUnstakeIndexList = new int256[](length);

    for (uint256 i = 0; i < length; ++i) {
        unstakeIndexList[i] = unstakesOfUser[msg.sender].at(i);
    }
    uint256 curEra = currentEra();
    for (uint256 i = 0; i < length; ++i) {
        uint256 unstakeIndex = unstakeIndexList[i];
        UnstakeInfo memory unstakeInfo = unstakeAtIndex[unstakeIndex];
        if (unstakeInfo.era.add(unbondingDuration) > curEra || unstakeInfo.pool != _poolAddress) {
            emitUnstakeIndexList[i] = -1;
            continue;
        }

        require(unstakesOfUser[msg.sender].remove(unstakeIndex), "already withdrawed");

        totalWithdrawAmount = totalWithdrawAmount.add(unstakeInfo.amount);
        emitUnstakeIndexList[i] = int256(unstakeIndex);
    }

    if (totalWithdrawAmount > 0) {
        IStakePool(_poolAddress).withdrawForStaker(erc20TokenAddress, msg.sender, totalWithdrawAmount);
    }

    emit Withdraw(msg.sender, _poolAddress, totalWithdrawAmount, emitUnstakeIndexList);
}

// ----- permissionless

function newEra() external {
    uint256 _era = latestEra.add(1);
    require(currentEra() >= _era, "calEra not match");

    // update era
    latestEra = _era;

    uint256 totalNewReward;
    uint256 newTotalActive;
    address[] memory poolList = getBondedPools();
    for (uint256 i = 0; i < poolList.length; ++i) {
        address poolAddress = poolList[i];

        uint256[] memory validators = getValidatorIdsOf(poolAddress);

        // newReward
        uint256 poolNewReward = IStakePool(poolAddress).checkAndWithdrawRewards(validators);
        emit NewReward(poolAddress, poolNewReward);
        totalNewReward = totalNewReward.add(poolNewReward);

        // unstakeClaimTokens
        for (uint256 j = 0; j < validators.length; ++j) {
            uint256 oldClaimedNonce = maxClaimedNonceOf[poolAddress][validators[j]];
            uint256 newClaimedNonce = IStakePool(poolAddress).unstakeClaimTokens(validators[j], oldClaimedNonce);
            if (newClaimedNonce > oldClaimedNonce) {
                maxClaimedNonceOf[poolAddress][validators[j]] = newClaimedNonce;

                emit NewClaimedNonce(poolAddress, validators[j], newClaimedNonce);
            }
        }

        // bond or unbond
        PoolInfo memory poolInfo = poolInfoOf[poolAddress];
        uint256 poolBondAndNewReward = poolInfo.bond.add(poolNewReward);
        if (poolBondAndNewReward > poolInfo.unbond) {
            uint256 needDelegate = poolBondAndNewReward.sub(poolInfo.unbond);
            IStakePool(poolAddress).delegate(validators[0], needDelegate);

            emit Delegate(poolAddress, validators[0], needDelegate);
        } else if (poolBondAndNewReward < poolInfo.unbond) {
            uint256 needUndelegate = poolInfo.unbond.sub(poolBondAndNewReward);

            for (uint256 j = 0; j < validators.length; ++j) {
                if (needUndelegate == 0) {
                    break;
                }
                uint256 totalStaked = IStakePool(poolAddress).getTotalStakeOnValidator(validators[j]);

                uint256 unbondAmount;
                if (needUndelegate < totalStaked) {
                    unbondAmount = needUndelegate;
                    needUndelegate = 0;
                } else {
                    unbondAmount = totalStaked;
                    needUndelegate = needUndelegate.sub(totalStaked);
                }

                if (unbondAmount > 0) {
                    IStakePool(poolAddress).undelegate(validators[j], unbondAmount);

                    emit Undelegate(poolAddress, validators[j], unbondAmount);
                }
            }
            require(needUndelegate == 0, "undelegate not enough");
        }

        // cal total active
        uint256 newPoolActive = IStakePool(poolAddress).getTotalStakeOnValidators(validators);
        newTotalActive = newTotalActive.add(newPoolActive);

        // update pool state
        poolInfo.active = newPoolActive;
        poolInfo.bond = 0;
        poolInfo.unbond = 0;

        poolInfoOf[poolAddress] = poolInfo;
    }

    // cal protocol fee
    if (totalNewReward > 0) {
        uint256 rTokenProtocolFee = totalNewReward.mul(protocolFeeCommission).div(rate);
        if (rTokenProtocolFee > 0) {
            totalProtocolFee = totalProtocolFee.add(rTokenProtocolFee);
            // mint rtoken
            totalRTokenSupply = totalRTokenSupply.add(rTokenProtocolFee);
            IERC20MintBurn(rTokenAddress).mint(address(this), rTokenProtocolFee);
        }
    }

    // update rate
    uint256 newRate = newTotalActive.mul(1e18).div(totalRTokenSupply);
    uint256 rateChange = newRate > rate ? newRate.sub(rate) : rate.sub(newRate);
    require(rateChange.mul(1e18).div(rate) < rateChangeLimit, "rate change over limit");

    rate = newRate;
    eraRate[_era] = newRate;

    emit ExecuteNewEra(_era, newRate);
}

}

Tore19 commented 1 year ago
  1. Potential for Front-Running

Actually, there is a front-running problem in the external transaction of any contract, but whether there is an attack depends on the specific function. About the stake and unstake functions, there are no issues with front-running attacks.

  1. Reentrancy

Same to 1, it depends on the specific function. According to our assessment, there are no issues with reentrancy attacks.

  1. Gas Usage

In actual use, the length of these two arrays is very limited, and there will be no gas problem.

  1. Please check setParams function, which can update the protocolFeeCommission, rateChangeLimit, unbondingDuration and other such parameters.