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: 11/15
Findings: 3
Award: $1,619.82
🌟 Selected for report: 1
🚀 Solo Findings: 0
pants
Non-standard compliant tokens like USDT don't return a return value on approve(). The function Erc20.approve() reverts if the token doesn't return a return value, as it is defined to always return a boolean.
When using any non-standard compliant token like USDT, the function will revert unexpectedly.
Manual code review.
Use OpenZeppelin’s SafeERC20.safeApprove() that handles the return value check as well as non-standard compliant tokens.
pants
setSwivelAddress function doesn't check the admin is address(0).
#0 - 0xean
2021-10-16T23:07:56Z
dupe of #100
pants
The file MarketPlace.sol has an open TODO:
// TODO update to 0.8.4 (or whatever latest is...) // NOTE the pattern [underlying, maturity*, cToken, ...] pragma solidity 0.8.4;
The function MarketPlace.createMarket() has an open TODO as well.
// TODO can we live with the factory pattern here both bytecode size wise and CREATE opcode cost wise? address zctAddr = address(new ZcToken(u, m, n, s, d)); address vAddr = address(new VaultTracker(m, c, swivel));
Open TODOs can hint at programming or architectural errors that still need to be fixed.
Manual code review.
Resolve the TODOs and bubble up the error.
🌟 Selected for report: JMukesh
Also found by: GalloDaSballo, pants
0.0893 ETH - $264.61
pants
There is no Admin transfer pattern at all.
Manual Code Checking
#0 - JTraversa
2021-10-07T17:06:18Z
I dont believe this necessarily is an issue. We have separate plans to add further admin functionality however left it out of the scope of this audit as not having a transferAdmin function is no more dangerous than the admin having the functionality it currently has.
#1 - 0xean
2021-10-16T23:15:35Z
dupe of #119
pants
Operations happens in a loop must be gas efficient as possible. We recommend to: Change line 59 to for (uint256 i=0; i < o.length;) { And add a line between 76-77 with unchecked{i++}; Link to code - https://github.com/Swivel-Finance/gost/blob/v2/test/swivel/Swivel.sol#L59
#0 - 0xean
2021-10-16T22:35:50Z
dupe of #15
🌟 Selected for report: pants
0.0941 ETH - $278.98
pants
At both line 59 and 61 the code reads o[i]. It happens inside a loop therefore the best practice which is more gas efficient is to cache o[i] once and use it instead.
https://github.com/Swivel-Finance/gost/blob/v2/test/swivel/Swivel.sol#L59
Manual code review.
#0 - JTraversa
2021-10-24T20:14:33Z
https://github.com/Swivel-Finance/gost/commit/ed8b333b6440cc1b346086a279ae39c546516625
I'm unsure this is actually a savings is it? You mstore and mload instead of just referencing calldata?
Would appreciate judge comment real quick.
#1 - JTraversa
2021-11-03T21:34:17Z
Can confirm, saves ~1k gas per order