Assets are transferred if the execute call fails

[code423n4]

Handle

shw

Vulnerability details

Impact

When the user calls fulfill with a non-zero callTo parameter, the TransactionManager tries to call execute on callTo, and if the function call fails, the manager transfers toSend amount of receiving assets to receivingAddress. However, since the assets may have been transferred before (when the addFunds call fails), the user could get twice the toSend amount assets as a result.

Proof of Concept

Referenced code:
TransactionManager.sol#L405-L408
TransactionManager.sol#L424-L427

Recommended Mitigation Steps

Since the assets are approved or transferred to the receivingAddress before calling the execute function, the manager should not transfer assets again if the execute call fails.

[LayneHaber]

#46