vkonst / ctd-token

Smart-contracts for Cointed Token and Token Sale
11 stars 9 forks source link

function Approve in StandardToken.sol #2

Closed pauliax closed 6 years ago

pauliax commented 6 years ago

Beware that changing an allowance with this method brings the risk described here: https://docs.google.com/document/d/1YLPtQxZu1UAvO9cZ1O2RPXBbT0mooh4DYKjA_jp-RLM/edit#heading=h.m9fhqynw2xvt One possible solution to mitigate this race condition is to first reduce the spender's allowance to 0 and set the desired value afterwards: require((_value == 0) || (allowed[msg.sender][_spender] == 0));

vkonst commented 6 years ago

Considered as code enhancement Code updated as suggested

The attack is well known and documented.

This is attack on API itself, not on any particular implementations, so all conformant implementations are potentially vulnerable.

To address the issue, the code incorporates increaseApproval and decreaseApproval methods. These methods protect from the said attack and therefor users shall prefer these methods over the approve in case they wish to change the previously approved amount. Cointed publishes this info for users.

However, we added the the suggested code to the approve method: require((_value == 0) || (allowed[msg.sender][_spender] == 0));

Thanks for your input.