Dangerous stake method usage

[k06a]

Hi, approveAndCall method is not in ERC20 and potentially some unofficial clients (web/mobile/desktop) may try to make 2 transactions as well as stake method is public:

1. The first Tx to approve bank some amount of tokens
2. The second Tx to call stake public method, which will transferFrom previously approved tokens

And anyone will be able to spend anyone tokens right between this transaction. If you're sure everybody will everytime use approveAndCall so why do you need stake method at all? Why do you need it even exist?

[ameensol]

We put stake in the contract specifically for third-party UIs which prefer to use the approve -> stake flow over approveAndCall.

And anyone will be able to spend anyone tokens right between this transaction

I don't think this is true, only the SpankBank would be approved, and the only way to get the SpankBank to trigger a transferFrom on the SPANK contract is to call stake.

[k06a]

@ameensol I will be able to call receiveApproval manually for your approve and I will be able to setup your stake.

[k06a]

I will be able to set any arguments I want for YOUR stake:

• uint256 spankAmount
• uint256 stakePeriods
• address delegateKey
• address bootyBase

[k06a]

@ameensol let's reopen the issue :)

[k06a]

Someone could use this scheme to intervene your users to work with spankbank.

[k06a]

I'd recommend looking at ERC1003 for solution fully compatible with ERC20 two-transaction mode: ethereum/EIPs#1003

[k06a]

Looks like stacked BOOTY can be stolen in case of setting hackers bootyBase address

[ameensol]

This is a good catch.

I will be able to call receiveApproval manually for your approve and I will be able to setup your stake.

You would indeed be able to do this.

    function receiveApproval(address from, uint256 amount, address tokenContract, bytes extraData) SpankBankIsOpen public returns (bool success) {
        address delegateKeyFromBytes = extraData.toAddress(12);
        address bootyBaseFromBytes = extraData.toAddress(44);
        uint256 periodFromBytes = extraData.toUint(64);

        emit ReceiveApprovalEvent(from, tokenContract);

        doStake(from, amount, periodFromBytes, delegateKeyFromBytes, bootyBaseFromBytes);
        return true;
    }

This could be solved by checking inside the receiveApproval function that the msg.sender is the BOOTY ERC20 contract (bootyToken.address).

[k06a]

Also hacker can vote for closing for some of the participants. Trying to enumerate all possible attack vectors.

[ameensol]

Hey, we fixed this as part of a more critical bugfix.

https://medium.com/spankchain/spankbank-hotfix-5bca5bfd288d

Send me an ETH address and I'll send you $1,000 worth of ETH as a reward.

Thanks!