Platform: Code4rena
Start Date: 30/09/2021
Pot Size: $75,000 ETH
Total HM: 9
Participants: 15
Period: 7 days
Judge: 0xean
Total Solo HM: 2
Id: 39
League: ETH
Rank: 14/15
Findings: 3
Award: $1,229.67
🌟 Selected for report: 2
🚀 Solo Findings: 0
GalloDaSballo
initiateZcTokenFillingZcTokenExit and initiateVaultFillingVaultExit in Swivel.sol are using transferFrom
https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/swivel/Swivel.sol#L191
https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/swivel/Swivel.sol#L253
This function call can fail (meaning the tokens are not transferred) without causing a revert
This would break the accounting in the protocol
I highly recommend using OpenZeppelin's safeTransferFrom to avoid issue
See a similar finding in Pool Together audit: https://github.com/code-423n4/2021-07-pooltogether-findings/issues/61
Replace transferFrom with safeTransferFrom
GalloDaSballo
The constructor in Swivel.sol receives a parameter m: https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/swivel/Swivel.sol#L42
There is no checks to avoid the address is 0
Add
require(m != address(0))
🌟 Selected for report: JMukesh
Also found by: GalloDaSballo, pants
GalloDaSballo
While not a vulnerability, you may want to change the admin key to be burnable
This can be helpful as you decentralized the protocol or reduce centralized governance
Add a function to renounce admin
#0 - JTraversa
2021-10-08T05:20:40Z
Placing this as a duplicate part of a larger transferAdmin ticket
#1 - 0xean
2021-10-16T23:19:42Z
dupe of #119
🌟 Selected for report: GalloDaSballo
0.0941 ETH - $278.98
GalloDaSballo
The function custodialExit in Marketplace.sol: https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/marketplace/MarketPlace.sol#L194
Always returns true or reverts
In swivel.sol the check
require(mPlace.custodialExit(o.underlying, o.maturity, o.maker, msg.sender, a), 'custodial exit failed');
will always pass, unless the function custodialExit reverts
This extra require is not necessary, and provides no additional guarantees as custodialExit will always return true or revert
Replace
require(mPlace.custodialExit(o.underlying, o.maturity, o.maker, msg.sender, a), 'custodial exit failed');
With
mPlace.custodialExit(o.underlying, o.maturity, o.maker, msg.sender, a)
#0 - JTraversa
2021-10-24T19:20:37Z
🌟 Selected for report: GalloDaSballo
0.0941 ETH - $278.98
GalloDaSballo
The line: require(MarketPlace(marketPlace).p2pZcTokenExchange(o.underlying, o.maturity, o.maker, msg.sender, a), 'zcToken exchange failed');
in Swivel.sol https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/swivel/Swivel.sol#L171
Is checking for the return value of MarketPlace.p2pZcTokenExchange, however p2pZcTokenExchange will always return true or revert
As such the require is not necessary, and doesn't provide any additional guarantees.
Replace
require(MarketPlace(marketPlace).p2pZcTokenExchange(o.underlying, o.maturity, o.maker, msg.sender, a), 'zcToken exchange failed');
With
MarketPlace(marketPlace).p2pZcTokenExchange(o.underlying, o.maturity, o.maker, msg.sender, a)
#0 - JTraversa
2021-10-24T19:20:22Z