QA Report

[code423n4]

Low

Admin can accidentally be set to an invalid address, resulting in loss of control of the Swivel contract.

The current admin of the swivel contract can accidentally transfer ownership to an invalid adddress.

Recommended mitigation:

• Consider two-phase ownership transfer to prevent sending ownership to invalid address.

There is 1 instance of this issue:

FILE: Swivel/Swivel.sol

427  /// @param a Address of a new admin
428  function setAdmin(address a) external authorized(admin) returns (bool) {
429    admin = a;
430
431    return true;
432: }

https://github.com/code-423n4/2022-07-swivel/blob/main/Swivel/Swivel.sol#L427-L432

approveUnderlying reverts prematurely after encountering the first unapproved underlying token

The function approveUnderlying reverts prematurely when the for-loop encounters the first unapproved underlying token when bulk-approving compound token. This makes the approveUnderlying function useless if there are any unapproved underlying tokens.

Low severity as this function is for convenience, and the same operations can be performed using other contract functions.

Recommended mitigation:

• Use an if statement to run lines 560-565 when the underlying token is approved, otherwise do nothing.

There is 1 instance of this issue:

FILE: Swivel/Swivel.sol

552    for (uint256 i; i < len;) {
553      when = approvals[u[i]];
554
555      if (when == 0) { revert Exception(16, 0, 0, address(0), address(0)); }
556
557      if (block.timestamp < when) { revert Exception(17, block.timestamp, when, address(0), address(0));
558      }
559
560      approvals[u[i]] = 0;
561      IErc20 uToken = IErc20(u[i]);
562      Safe.approve(uToken, c[i], max);
563      unchecked {
564        i++;
565     }
566:   }

https://github.com/code-423n4/2022-07-swivel/blob/main/Swivel/Swivel.sol#L552-L566

Non-critical

Incorrect comments

There are 2 instances of this issue:

FILE: Swivel/Swivel.sol

482    /// @notice Emergency function to block unplanned withdrawals // @audit should be "..block unplanned fee change".
483:  function blockFeeChange() external authorized(admin) returns (bool) {

https://github.com/code-423n4/2022-07-swivel/blob/main/Swivel/Swivel.sol#L482-L483

FILE: Swivel/Swivel.sol

531  /// @notice Emergency function to block unplanned withdrawals // @audit should be "... block unplanned approvals".
532  /// @param e Address of token approval to block
533: function blockApproval(address e) external authorized(admin) returns (bool) {

https://github.com/code-423n4/2022-07-swivel/blob/main/Swivel/Swivel.sol#L531-L533

[robrobbins]

why would the approve fail? are you talkin overflow / underflow? seems unlikely.

even if it did I believe we want all or nothing. in if around it would possible put the end state into some partial success wierdness