Platform: Code4rena
Start Date: 24/06/2021
Pot Size: $80,000 USDC
Total HM: 18
Participants: 12
Period: 7 days
Judge: cemozer
Total Solo HM: 11
Id: 16
League: ETH
Rank: 8/12
Findings: 3
Award: $2,191.84
🌟 Selected for report: 4
🚀 Solo Findings: 0
JMukesh
return value from transfer()/transferFrom ensure success of the call and if not , it describe the reason for. It is usually good to add a require-statement that checks the return value or to use something like safeTransfer; unless one is sure the given token reverts in case of a failure.
reference --> https://consensys.net/diligence/audits/2021/01/fei-protocol/#unchecked-return-value-for-iweth-transfer-call
manual review
use safeTransfer()/SafeTransferFrom
#0 - raymogg
2021-07-05T03:25:21Z
Duplicate of #115
JMukesh
fund can be lost, since state variable are initialised in constructor are used in various function , error in this can lead to redeployment of contract
manual review
add require condition to check inputs of constructor
#0 - raymogg
2021-07-05T23:42:00Z
Duplicate of #136
JMukesh
https://swcregistry.io/docs/SWC-103
manual review
use fixed solidity version
#0 - raymogg
2021-07-05T23:17:47Z
Duplicate of #133
🌟 Selected for report: JMukesh
671.0176 USDC - $671.02
JMukesh
These array memory parameter can be problematic if not used properly , if the array is very large it may overlap over other part of memory.
This an example to show the exploit: // based on https://github.com/paradigm-operations/paradigm-ctf-2021/blob/master/swap/private/Exploit.sol pragma solidity ^0.4.24; // only works with low solidity version
contract test{ struct Overlap { uint field0; } event log(uint);
function mint(uint[] memory amounts) public returns (uint) { // this can be in any solidity version Overlap memory v; v.field0 = 1234; emit log(amounts[0]); // would expect to be 0 however is 1234 return 1; }
function go() public { // this part requires the low solidity version uint x=0x800000000000000000000000000000000000000000000000000000000000000; // 2^251 bytes memory payload = abi.encodeWithSelector(this.mint.selector, 0x20, x); bool success=address(this).call(payload); } }
manual review
check array length before using it
#0 - raymogg
2021-07-05T23:10:24Z
Duplicate of #79
#1 - sporejack
2021-07-07T00:42:51Z
So the provided PoC code works (under solc 0.4.24) subject to test case:
const { expect } = require("chai"); describe("Overlap", async () => { describe("go", async () => { let overlapFactory; let overlap; before(async () => { overlapFactory = await ethers.getContractFactory("Overlap"); overlap = await overlapFactory.deploy(); await overlap.deployed(); }); context("When called", async () => { it("Emits `log` event with correct value", async () => { var firstAmount = ethers.BigNumber.from("1234"); await expect(overlap.go()).to.emit(overlap, "log").withArgs(firstAmount); }); }); }); });
#2 - sporejack
2021-07-07T00:45:28Z
My assessment is:
| Impact | Difficulty | Overall |
|---|---|---|
| Low | Low | Low |
With rationale:
🌟 Selected for report: JMukesh
333.3333 USDC - $333.33
JMukesh
Unused state variable will increase unnecessarily code size and use the memory
manual review
remove the variable which are unused
🌟 Selected for report: JMukesh
333.3333 USDC - $333.33
JMukesh
state variable which have to initialise in constructor can be declared as immutable to save gas
https://docs.soliditylang.org/en/v0.8.4/contracts.html#constant-and-immutable-state-variables
manual review
#0 - OsmanBran
2021-07-19T03:34:54Z
Duplicate of #24
#1 - loudoguno
2021-08-24T17:29:40Z
reopening and removing duplicate label as per judges sheet
🌟 Selected for report: JMukesh
333.3333 USDC - $333.33
JMukesh
public function which are not called within contract should be declared as external to save gas
manual review
Declare public function as external which are not called in the contract