Swivel v3 contest - kyteg's results

The Capital-Efficient Protocol For Fixed-Rate Lending.

General Information

Platform: Code4rena

Start Date: 12/07/2022

Pot Size: $35,000 USDC

Total HM: 13

Participants: 78

Period: 3 days

Judge: 0xean

Total Solo HM: 6

Id: 135

League: ETH

Swivel

Findings Distribution

Researcher Performance

Rank: 29/78

Findings: 2

Award: $106.28

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest Page Github Contest Repository Github Findings Repository Swivel

Findings Information

🌟 Selected for report: joestakey

Also found by: 0x1f8b, 0x52, 0xDjango, 0xNazgul, 0xNineDec, 8olidity, Avci, Bahurum, Bnke0x0, Chom, ElKu, Funen, GimelSec, JC, Junnon, Kaiziron, Meera, PaludoX0, Picodes, ReyAdmirado, Sm4rty, Soosh, Waze, _Adam, __141345__, ak1, aysha, benbaessler, bin2chen, c3phas, cccz, cryptphi, csanuragjain, defsec, exd0tpy, fatherOfBlocks, gogo, hake, hansfriese, itsmeSTYJ, jonatascm, kyteg, mektigboy, oyc_109, pashov, rbserver, rishabh, robee, rokinot, sach1r0, sashik_eth, scaraven, simon135, slywaters

Awards

45.5501 USDC - $45.55

Labels

bug

duplicate

QA (Quality Assurance)

wontfix

External Links

Link to GitHub issue

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

#0 - robrobbins
2022-08-29T22:49:54Z

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

Findings Information

🌟 Selected for report: joestakey

Also found by: 0x040, 0x1f8b, 0xDjango, 0xNazgul, 0xsam, Avci, Aymen0909, Bnke0x0, CRYP70, ElKu, Fitraldys, Funen, JC, Kaiziron, MadWookie, Meera, ReyAdmirado, Sm4rty, Soosh, TomJ, Waze, _Adam, __141345__, ajtra, benbaessler, c3phas, csanuragjain, durianSausage, exd0tpy, fatherOfBlocks, hake, ignacio, karanctf, kyteg, m_Rassska, oyc_109, rbserver, robee, rokinot, samruna, sashik_eth, simon135, slywaters

Awards

60.7274 USDC - $60.73

Labels

bug

duplicate

G (Gas Optimization)

resolved

sponsor confirmed

External Links

Link to GitHub issue

Summary

Issue	Instances
`++i` uses less gas compared to `i++`	5
`uint8` incurs more gas overhead compared to `uint256`	7
Use named return variable instead of using return	2
Named return variable is not used	8
Use != 0 instead of > 0 for a `uint`	17

Gas Optimizations

`++i` uses less gas compared to `i++`

This is especially relevant for the use of i++ in for loops. This saves 6 gas per loop.

There are 5 instances of this issue:

FILE: Swivel/Swivel.sol

100:   unchecked {i++;}

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

FILE: Swivel/Swivel.sol

269:    unchecked {i++;}

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

FILE: Swivel/Swivel.sol

417      unchecked {
418        i++;
419:      }

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

FILE: Swivel/Swivel.sol

510      unchecked {
511        x++;
512:      }

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

FILE: Swivel/Swivel.sol

563      unchecked {
564        i++;
565:      }

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

`uint8`, `uint16`, incurs more gas overhead compared to `uint256`

There are 7 instances of this issue:

File: Swivel/Swivel.sol

35:   uint16 constant public MIN_FEENOMINATOR = 33;

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

File: Swivel/Swivel.sol

37:   uint16[4] public feenominators;

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

FILE: Marketplace/MarketPlace.sol

65:   uint8 p,

https://github.com/code-423n4/2022-07-swivel/blob/main/Marketplace/MarketPlace.sol#L65

FILE: Creator/Creator.sol

31:   uint8 p,

https://github.com/code-423n4/2022-07-swivel/blob/main/Creator/Creator.sol#L31

FILE: Creator/Creator.sol

38:   uint8 d

https://github.com/code-423n4/2022-07-swivel/blob/main/Creator/Creator.sol#L38

FILE: VaultTracker/VaultTracker.sol

26:   uint8 public immutable protocol;

https://github.com/code-423n4/2022-07-swivel/blob/main/VaultTracker/VaultTracker.sol#L26

FILE: Creator/ZcToken.sol

17:   uint8 public immutable protocol;

https://github.com/code-423n4/2022-07-swivel/blob/main/Creator/ZcToken.sol#L17

Use named return variable instead of using return

There are 2 instances of this issue:

FILE: Swivel/Swivel.sol

697:  return hash;

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

FILE: Swivel/Swivel.sol

207:    return interest;

https://github.com/code-423n4/2022-07-swivel/blob/main/Marketplace/MarketPlace.sol#L207

Named return variable is not used

Remove the named return variable to save 25 gas per function call.

There are 8 instances of this issue:

FILE: Marketplace/Marketplace.sol

148:     function authRedeem(uint8 p, address u, uint256 m, address f, address t, uint256 a) public authorized(markets[p][u][m].zcToken) returns (uint256 underlyingAmount) {

https://github.com/code-423n4/2022-07-swivel/blob/main/Marketplace/MarketPlace.sol#L148

FILE: Creator/ZcToken.sol

43:    function convertToUnderlying(uint256 principalAmount) external override view returns (uint256 underlyingAmount){

https://github.com/code-423n4/2022-07-swivel/blob/main/Creator/ZcToken.sol#L43

FILE: Creator/ZcToken.sol

52:   function convertToPrincipal(uint256 underlyingAmount) external override view returns (uint256 principalAmount){

https://github.com/code-423n4/2022-07-swivel/blob/main/Creator/ZcToken.sol#L52

FILE: Creator/ZcToken.sol

61:   function maxRedeem(address owner) external override view returns (uint256 maxPrincipalAmount){

https://github.com/code-423n4/2022-07-swivel/blob/main/Creator/ZcToken.sol#L61

FILE: Creator/ZcToken.sol

70:    function previewRedeem(uint256 principalAmount) external override view returns (uint256 underlyingAmount){

https://github.com/code-423n4/2022-07-swivel/blob/main/Creator/ZcToken.sol#L70

FILE: Creator/ZcToken.sol

79:   function maxWithdraw(address owner) external override view returns (uint256 maxUnderlyingAmount){

https://github.com/code-423n4/2022-07-swivel/blob/main/Creator/ZcToken.sol#L79

FILE: Creator/ZcToken.sol

88:     function previewWithdraw(uint256 underlyingAmount) external override view returns (uint256 principalAmount){

https://github.com/code-423n4/2022-07-swivel/blob/main/Creator/ZcToken.sol#L88

FILE: Creator/ZcToken.sol

98:     function withdraw(uint256 underlyingAmount, address receiver, address holder) external override returns (uint256 principalAmount){

https://github.com/code-423n4/2022-07-swivel/blob/main/Creator/ZcToken.sol#L98

FILE: Creator/ZcToken.sol

124:     function redeem(uint256 principalAmount, address receiver, address holder) external override returns (uint256 underlyingAmount){

https://github.com/code-423n4/2022-07-swivel/blob/main/Creator/ZcToken.sol#L124

Use != 0 instead of > 0 for a `uint`

This saves 6 gas per instance.

There are 17 instances of this issue:

FILE: Swivel/Swivel.sol

118:    if (amount > o.premium) { revert Exception(5, amount, o.premium, address(0), address(0)); }

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

FILE: Swivel/Swivel.sol

155:   if (amount > o.principal) { revert Exception(5, amount, o.principal, address(0), address(0)); }

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

FILE: Swivel/Swivel.sol

190:   if (amount > o.principal) { revert Exception(5, amount, o.principal, address(0), address(0)); }

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

FILE: Swivel/Swivel.sol

219:   if (amount > o.premium) { revert Exception(5, amount, o.premium, address(0), address(0)); }

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

FILE: Swivel/Swivel.sol

284:  if (amount > o.premium) { revert Exception(5, amount, o.premium, address(0), address(0)); }

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

FILE: Swivel/Swivel.sol

315:   if (amount > o.principal) { revert Exception(5, amount, o.principal, address(0), address(0)); }

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

FILE: Swivel/Swivel.sol

345:   if (amount > o.principal) { revert Exception(5, amount, o.principal, address(0), address(0)); }

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

FILE: Swivel/Swivel.sol

380:   if (amount > o.premium) { revert Exception(5, amount, o.premium, address(0), address(0)); }

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

FILE: VaultTracker/VaultTracker.sol

54:    if (vlt.notional > 0) {

https://github.com/code-423n4/2022-07-swivel/blob/main/VaultTracker/VaultTracker.sol#L54

FILE: VaultTracker/VaultTracker.sol

59:   if (maturityRate > 0) { // Calculate marginal interest

https://github.com/code-423n4/2022-07-swivel/blob/main/VaultTracker/VaultTracker.sol#L59

FILE: VaultTracker/VaultTracker.sol

86:  if (a > vlt.notional) { revert Exception(31, a, vlt.notional, address(0), address(0)); }

https://github.com/code-423n4/2022-07-swivel/blob/main/VaultTracker/VaultTracker.sol#L86

FILE: VaultTracker/VaultTracker.sol

93:   if (maturityRate > 0) { // Calculate marginal interest

https://github.com/code-423n4/2022-07-swivel/blob/main/VaultTracker/VaultTracker.sol#L93

FILE: VaultTracker/VaultTracker.sol

123:   if (maturityRate > 0) { // Calculate marginal interest

https://github.com/code-423n4/2022-07-swivel/blob/main/VaultTracker/VaultTracker.sol#L123

FILE: VaultTracker/VaultTracker.sol

158:   if (a > from.notional) { revert Exception(31, a, from.notional, address(0), address(0)); }

https://github.com/code-423n4/2022-07-swivel/blob/main/VaultTracker/VaultTracker.sol#L158

FILE: VaultTracker/VaultTracker.sol

165:   if (maturityRate > 0) {

https://github.com/code-423n4/2022-07-swivel/blob/main/VaultTracker/VaultTracker.sol#L165

FILE: VaultTracker/VaultTracker.sol

181:   if (to.notional > 0) {

https://github.com/code-423n4/2022-07-swivel/blob/main/VaultTracker/VaultTracker.sol#L181

FILE: VaultTracker/VaultTracker.sol

184:   if (maturityRate > 0) {

https://github.com/code-423n4/2022-07-swivel/blob/main/VaultTracker/VaultTracker.sol#L184

FILE: VaultTracker/VaultTracker.sol

224:  if (maturityRate > 0) {

https://github.com/code-423n4/2022-07-swivel/blob/main/VaultTracker/VaultTracker.sol#L222

#0 - robrobbins
2022-08-31T20:03:56Z

some addressed elsewhere, or dupes or wontfixes... but...

setting to resolved as, tho nothing in here is unique to this ticket it may be that this one was seen by a sponsor first?

Swivel v3 contest - kyteg's results

The Capital-Efficient Protocol For Fixed-Rate Lending.

General Information

Findings Distribution

Researcher Performance

External Links

[Q-16] QA Report - $45.55

Findings Information

Awards

Labels

External Links

Low

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

approveUnderlying reverts prematurely after encountering the first unapproved underlying token

Non-critical

Incorrect comments

#0 - robrobbins2022-08-29T22:49:54Z

[G-40] Gas Report - $60.73

Findings Information

Awards

Labels

External Links

Summary

Gas Optimizations

++i uses less gas compared to i++

uint8, uint16, incurs more gas overhead compared to uint256

Use named return variable instead of using return

Named return variable is not used

Use != 0 instead of > 0 for a uint

#0 - robrobbins2022-08-31T20:03:56Z

`approveUnderlying` reverts prematurely after encountering the first unapproved underlying token

#0 - robrobbins
2022-08-29T22:49:54Z

`++i` uses less gas compared to `i++`

`uint8`, `uint16`, incurs more gas overhead compared to `uint256`

Use != 0 instead of > 0 for a `uint`

#0 - robrobbins
2022-08-31T20:03:56Z