Platform: Code4rena
Start Date: 16/09/2021
Pot Size: $50,000 USDC
Total HM: 26
Participants: 30
Period: 7 days
Judge: GalloDaSballo
Total Solo HM: 17
Id: 36
League: ETH
Rank: 14/30
Findings: 4
Award: $1,043.18
🌟 Selected for report: 3
🚀 Solo Findings: 0
131.4735 USDC - $131.47
JMukesh
return value from transfer() should be checked because ,it give the indication wether call is successful or not and different type of token handle the error differently so it is safe to use SafeTransfer()
manual review
use safeTransfer()
#0 - frank-beard
2021-10-19T16:57:48Z
#1 - GalloDaSballo
2021-12-19T22:15:51Z
Duplicate of #196
🌟 Selected for report: 0xalpharush
Also found by: JMukesh, hack3r-0m, johnsterlacci
JMukesh
In burn(), it does not check wether the recipient is EOA or contract addresss and tokens are burned after the pushUnderlying() which transfer the token , this can lead to reentrancy
some tokens allows the token contract to notify senders and recipients when tokens are sent or received from their accounts. This notification is in the form of a callback to the recipient. Therefore, if the recipient of the tokens is a smart contract, the smart contract can choose to react to such events. One possible reaction to such an event is reentering the contract .
manual review
add reentrancy guard
#0 - frank-beard
2021-09-28T21:28:54Z
#1 - GalloDaSballo
2021-12-19T00:04:42Z
Duplicate of #248
90.1739 USDC - $90.17
JMukesh
since the parameter of the constructor are used to initialize the sate variable and these state variable are used throughout the contract error in these parameter can lead to redeployment of the contract
manual review
add address(0) validation
#0 - GalloDaSballo
2021-11-30T23:13:07Z
Agree with finding
#1 - GalloDaSballo
2021-12-01T22:30:04Z
Duplicate of #86
🌟 Selected for report: JMukesh
333.9773 USDC - $333.98
JMukesh
In setOwnerSplit() ,
require(newOwnerSplit <= 2e17);
the newOwnerSplit can be equal to 20% , but in the docs it was mentioned that protocolOwner fee must be less than 20%
manual review
#0 - GalloDaSballo
2021-12-09T13:26:36Z
Valid finding as code is inconsistent with documentation
150.2898 USDC - $150.29
JMukesh
due to lack of checking of array parameters in settleAuction() , these array parameters can have different length which can lead to error. inputWeight is iterated over the length of inputToken if one of the parameter have less length than other one will become inaccessible which can lead to error
manual review
#0 - GalloDaSballo
2021-12-12T16:19:26Z
Agreed that a lack of checks for array length can cause undefined behaviour
150.2898 USDC - $150.29
JMukesh
by using approve() , we are not checking the value returned by the approve ,wether it got failed or successfully executed. so it is safe to use safeApproval()
manual review
use safeApprove()
#0 - frank-beard
2021-10-19T17:38:57Z
#1 - GalloDaSballo
2021-12-19T15:53:51Z
Duplicate of #35
#2 - GalloDaSballo
2021-12-19T15:54:36Z
Actually this is not a duplicate, as this is a finding that mentions to use safeApprove instead of the DOS of using a specific token This is a low risk finding
4.3799 USDC - $4.38
JMukesh
bool public override auctionOngoing; uint256 public override auctionStart; bool public override hasBonded; uint256 public override bondAmount; uint256 public override bondTimestamp; bool public override initialized;
the above state variable use 6 slot of memory , if we declare same type of variable in a row then they can be packed in one slot
uint256 public override auctionStart; uint256 public override bondAmount; uint256 public override bondTimestamp; bool public override auctionOngoing; bool public override hasBonded; bool public override initialized;
now the above state variable will take only 4 slot of memory because all three bool variable can be packed in the single slot
manual review
#0 - GalloDaSballo
2021-11-29T14:11:56Z
Duplicate of #109