Swivel v3 contest - oyc_109'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
Findings Distribution
Researcher Performance
Rank: 6/78
Findings: 4
Award: $2,358.52
🌟 Selected for report: 1
🚀 Solo Findings: 1
Findings Information
🌟 Selected for report: 0xDjango
Also found by: 0x1f8b, 8olidity, Bahurum, Lambda, arcoun, caventa, csanuragjain, hansfriese, joestakey, jonatascm, oyc_109, ronnyx2017
Lines of code
Vulnerability details
validation check on allowed amount wrong in withdraw function
description
the validation check if (allowed >= previewAmount) is incorrect as allowed should be greater than previewAmount
this will make withdraw always revert when using an allowance
if allowed < previouAmount the next line will revert
recommendation
change the if statement to require
#0 - JTraversa
2022-07-20T07:25:53Z
Duplicate of #129
#1 - bghughes
2022-08-03T14:43:57Z
Duplicate of #129
Findings Information
🌟 Selected for report: oyc_109
Labels
Awards
2234.6752 USDC - $2,234.68
External Links
Lines of code
https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/VaultTracker/VaultTracker.sol#L86 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/VaultTracker/VaultTracker.sol#L158
Vulnerability details
should use >= instead of >
description
https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/VaultTracker/VaultTracker.sol#L86 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/VaultTracker/VaultTracker.sol#L158
the comparison should be 'a >= vlt.notional' instead of a > vlt.notional
otherwise dust amounts will always be left in vlt.notional when calling removeNotional() or transferNotionalFrom()
#0 - JTraversa
2022-07-15T23:25:33Z
While the leakage is potentially ... 1 wei, its leakage nonetheless and a frontend would surely provide the max amount most of the time leading to reverts.
Maybe a quick severity review from wardens but happy with where its at.
#1 - bghughes
2022-08-03T02:04:04Z
While the leakage is potentially ... 1 wei, its leakage nonetheless and a frontend would surely provide the max amount most of the time leading to reverts.
Maybe a quick severity review from wardens but happy with where its at.
I agree that this is quite low stakes, given that there is non-zero potential leakage I think the warden should get credit for this as a Medium Risk vulnerability. Per the C4 juging criteria:
2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.
#2 - JTraversa
2022-08-05T00:38:17Z
@warden
#3 - robrobbins
2022-08-10T21:24:56Z
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
49.4308 USDC - $49.43
Labels
External Links
No Transfer Ownership Pattern
description
Recommend considering implementing a two step process where the owner or admin nominates an account and the nominated account needs to call an acceptOwnership() function for the transfer of ownership to fully succeed. This ensures the nominated EOA account is a valid and active account.
findings
https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Creator/Creator.sol#L47-L50 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Marketplace/MarketPlace.sol#L53-L56 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Swivel/Swivel.sol#L428-L432
missing checks for zero address
description
Checking addresses against zero-address during initialization or during setting is a security best-practice. However, such checks are missing in address variable initializations/changes in many places.
Impact: Allowing zero-addresses will lead to contract reverts and force redeployments if there are no setters for such address variables.
findings
https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Creator/Creator.sol#L48 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Creator/ZcToken.sol#L33-L37 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/VaultTracker/VaultTracker.sol#L35-L36 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Marketplace/MarketPlace.sol#L40 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Marketplace/MarketPlace.sol#L48
Unspecific Compiler Version Pragma
description
Avoid floating pragmas for non-library contracts.
While floating pragmas make sense for libraries to allow them to be included with multiple different versions of applications, it may be a security risk for application implementations.
A known vulnerable compiler version may accidentally be selected or security tools might fall-back to an older compiler version ending up checking a different EVM compilation that is ultimately deployed on the blockchain.
It is recommended to pin to a concrete compiler version.
findings
Use of Block.timestamp
description
Block timestamps have historically been used for a variety of applications, such as entropy for random numbers (see the Entropy Illusion for further details), locking funds for periods of time, and various state-changing conditional statements that are time-dependent. Miners have the ability to adjust timestamps slightly, which can prove to be dangerous if block timestamps are used incorrectly in smart contracts.
recommendation
Block timestamps should not be used for entropy or generating random numbers—i.e., they should not be the deciding factor (either directly or through some derivation) for winning a game or changing an important state.
Time-sensitive logic is sometimes required; e.g., for unlocking contracts (time-locking), completing an ICO after a few weeks, or enforcing expiry dates. It is sometimes recommended to use block.number and an average block time to estimate times; with a 10 second block time, 1 week equates to approximately, 60480 blocks. Thus, specifying a block number at which to change a contract state can be more secure, as miners are unable to easily manipulate the block number.
findings
https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Creator/ZcToken.sol#L44 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Creator/ZcToken.sol#L53 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Creator/ZcToken.sol#L62 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Creator/ZcToken.sol#L71 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Creator/ZcToken.sol#L80 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Creator/ZcToken.sol#L89 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Creator/ZcToken.sol#L101 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Creator/ZcToken.sol#L126 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Marketplace/MarketPlace.sol#L95 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Swivel/Swivel.sol#L437 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Swivel/Swivel.sol#L462 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Swivel/Swivel.sol#L691
Removing unused named returns variables can reduce gas usage (MSTOREs/MLOADs) and improve code clarity. To save gas and improve code quality: consider using only one of those.
findings
https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Marketplace/MarketPlace.sol#L158 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Marketplace/MarketPlace.sol#L166
admin can block withdraws
description
admin can block withdraws for an arbitrary address, users may lose trust in the protocol due to this level of control
recommend using the pausable modifier to stop unexpected withdraws instead
#0 - robrobbins
2022-08-31T00:53:20Z
dupes addressed elsewere
admin withdrawal blocking is likely not what you think. that method is strictly for swivel to withdraw its own, so the only thing blockable by an admin is his own scheduled withdraw
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
25.8577 USDC - $25.86
Labels
External Links
use calldata instead of memory
description
Use calldata instead of memory for function parameters saves gas if the function argument is only read.
findings
named returns and a return statement isn’t necessary
description
Removing unused named returns variables can reduce gas usage (MSTOREs/MLOADs) and improve code clarity. To save gas and improve code quality: consider using only one of those.
findings
https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Marketplace/MarketPlace.sol#L158 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Marketplace/MarketPlace.sol#L166
using prefix increments save gas
description
Prefix increments are cheaper than postfix increments, eg ++i rather than i++
findings
https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Swivel/Swivel.sol#L100 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Swivel/Swivel.sol#L269 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Swivel/Swivel.sol#L418 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Swivel/Swivel.sol#L511 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Swivel/Swivel.sol#L564