keep-core - TokenGrant and TokenStaking allow staking zero amount of tokens and front-running
Tokens are staked via the callback `receiveApproval()` which is normally invoked when calling `approveAndCall()`. The method is not restricting who can initiate the staking of tokens and relies on the fact that the token transfer to the `TokenStaking` contract is pre-approved by the owner, otherwise, the call would revert.\\nHowever, `receiveApproval()` allows the staking of a zero amount of tokens. The only check performed on the number of tokens transferred is, that the token holders balance covers the amount to be transferred. This check is both relatively weak - having enough balance does not imply that tokens are approved for transfer - and does not cover the fact that someone can call the method with a zero amount of tokens.\\nThis way someone could create an arbitrary number of operators staking no tokens at all. This passes the token balance check, `token.transferFrom()` will succeed and an operator struct with a zero stake and arbitrary values for `operator, from, magpie, authorizer` can be set. Finally, an event is emitted for a zero stake.\\nAn attacker could front-run calls to `receiveApproval` to block staking of a legitimate operator by creating a zero stake entry for the operator before she is able to. This vector might allow someone to permanently inconvenience an operator's address. To recover from this situation one could be forced to `cancelStake` terminating the zero stake struct in order to call the contract with the correct stake again.\\nThe same issue exists for `TokenGrant`.\\n```\\n/\\*\\*\\n \\* @notice Receives approval of token transfer and stakes the approved amount.\\n \\* @dev Makes sure provided token contract is the same one linked to this contract.\\n \\* @param \\_from The owner of the tokens who approved them to transfer.\\n \\* @param \\_value Approved amount for the transfer and stake.\\n \\* @param \\_token Token contract address.\\n \\* @param \\_extraData Data for stake delegation. This byte array must have the\\n \\* following values concatenated: Magpie address (20 bytes) where the rewards for participation\\n \\* are sent, operator's (20 bytes) address, authorizer (20 bytes) address.\\n \\*/\\nfunction receiveApproval(address \\_from, uint256 \\_value, address \\_token, bytes memory \\_extraData) public {\\n    require(ERC20Burnable(\\_token) == token, "Token contract must be the same one linked to this contract.");\\n    require(\\_value <= token.balanceOf(\\_from), "Sender must have enough tokens.");\\n    require(\\_extraData.length == 60, "Stake delegation data must be provided.");\\n\\n    address payable magpie = address(uint160(\\_extraData.toAddress(0)));\\n    address operator = \\_extraData.toAddress(20);\\n    require(operators[operator].owner == address(0), "Operator address is already in use.");\\n    address authorizer = \\_extraData.toAddress(40);\\n\\n    // Transfer tokens to this contract.\\n    token.transferFrom(\\_from, address(this), \\_value);\\n\\n    operators[operator] = Operator(\\_value, block.number, 0, \\_from, magpie, authorizer);\\n    ownerOperators[\\_from].push(operator);\\n\\n    emit Staked(operator, \\_value);\\n}\\n```\\n
Require tokens to be staked and explicitly disallow the zero amount of tokens case. The balance check can be removed.\\nNote: Consider checking the calls return value or calling the contract via `SafeERC20` to support potentially broken tokens that do not revert in error cases (token.transferFrom).
```\\n/\\*\\*\\n \\* @notice Receives approval of token transfer and stakes the approved amount.\\n \\* @dev Makes sure provided token contract is the same one linked to this contract.\\n \\* @param \\_from The owner of the tokens who approved them to transfer.\\n \\* @param \\_value Approved amount for the transfer and stake.\\n \\* @param \\_token Token contract address.\\n \\* @param \\_extraData Data for stake delegation. This byte array must have the\\n \\* following values concatenated: Magpie address (20 bytes) where the rewards for participation\\n \\* are sent, operator's (20 bytes) address, authorizer (20 bytes) address.\\n \\*/\\nfunction receiveApproval(address \\_from, uint256 \\_value, address \\_token, bytes memory \\_extraData) public {\\n    require(ERC20Burnable(\\_token) == token, "Token contract must be the same one linked to this contract.");\\n    require(\\_value <= token.balanceOf(\\_from), "Sender must have enough tokens.");\\n    require(\\_extraData.length == 60, "Stake delegation data must be provided.");\\n\\n    address payable magpie = address(uint160(\\_extraData.toAddress(0)));\\n    address operator = \\_extraData.toAddress(20);\\n    require(operators[operator].owner == address(0), "Operator address is already in use.");\\n    address authorizer = \\_extraData.toAddress(40);\\n\\n    // Transfer tokens to this contract.\\n    token.transferFrom(\\_from, address(this), \\_value);\\n\\n    operators[operator] = Operator(\\_value, block.number, 0, \\_from, magpie, authorizer);\\n    ownerOperators[\\_from].push(operator);\\n\\n    emit Staked(operator, \\_value);\\n}\\n```\\n