Token is not ERC20 compliant (potential DOS of other contracts by oracle)

[code423n4]

Handle

TomFrench

Vulnerability details

Impact

calling transferFrom with amount will reduce the callers allowance by an amount other than amount.
This under some circumstances can result in funds being locked in external contracts which perform exact approvals.

Proof of Concept

As a rebasing wrapper for ibBTC, wibBTC's transferFrom converts the passed amount into terms of the underlying shares to perform the transfer. approve however does not do this which opens up a potential issue.

// allowance[spender] = 0
wibBTC.approve(spender, amount)
// allowance[spender] = amount
wibBTC.approve(spender, recipient, amount)
// allowance[spender] = amount - amount.mul(1e18).div(pricePerShare)

As pricePerShare is expected to be greater than 1 then this is unlikely to cause any major problems - the worst that will happen is there's some unexpected leftover allowance and we don't get an expected gas refund.

However should pricePerShare < 1 (through loss of assets or a malicious oracle) then this code will start reverting and could potentially lock some wibBTC in contracts which rely on this behaviour until pricePerShare > 1 is once again true.

Recommended Mitigation Steps

Override approve function and add similar logic to that in transfer, update transferFrom to reduce allowance by amount rather than amountInShares

[0xleastwood]

#43