[WP-L5] Use of deprecated `safeApprove`

[code423n4]

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2022-01-sandclock/blob/a90ad3824955327597be00bb0bd183a9c228a4fb/sandclock/contracts/strategy/BaseStrategy.sol#L108-L109

ustToken.safeApprove(_ethAnchorRouter, type(uint256).max);
aUstToken.safeApprove(_ethAnchorRouter, type(uint256).max);

https://github.com/code-423n4/2022-01-sandclock/blob/a90ad3824955327597be00bb0bd183a9c228a4fb/sandclock/contracts/strategy/NonUSTStrategy.sol#L55-L56

ustToken.safeApprove(_curvePool, type(uint256).max);
underlying.safeApprove(_curvePool, type(uint256).max);

safeApprove is deprecated, see this comment.

Recommendation

As per OpenZepplin documentation:

whenever possible, use safeIncreaseAllowance and safeDecreaseAllowance instead

[naps62]

This is a non-issue, as confirmed directly with the reporter on discord

Quote of what I asked (and reporter acknowledged):

the reason these are deprecated, as far as I can tell, is because they suffer from the same front-running issues as regular ERC20 approvals. is that correct? if so, isn't this a non-issue in this case? There's nothing to front-run here, unless EthAnchor's router or the curve pool itself were malicious or even then, these approvals happen only at the constructor level, and for an infinite (sort of) amount

[r2moon]

This is duplicated with https://github.com/code-423n4/2022-01-sandclock-findings/issues/24