QA Report

[code423n4]

[L-01] Use safeTransfer/safeTransferFrom consistently

A require() statement that checks the return value of token transfers should be used to prevent silent transfer failures with non-compliant ERC20 tokens. Failure to do so can cause silent failures of transfers and affect token accounting in contract, possibly causing loss of value for the user or the protocol.

Proof of Concept

One unsafe transfer() exists without return value checks: https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/MerkleVesting.sol#L173

Recommended Mitigation Steps

Use require statements to check return values consistently or replace transfer/transferFrom with safeTransfer/safeTransferFrom from Open Zeppelin's SafeERC20 library

[L-02] isContract logic can be bypassed

The logic in isContract can be bypassed. It uses the same approached described in https://solidity-by-example.org/hacks/contract-size

Proof of Concept

The logic that can be bypassed is for isContract https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/VoterID.sol#L343

Recommended Mitigation Steps

One solution that works for now is to check tx.origin == msg.sender. This is true for EOAs and false for contracts, but this can change with later EIPs.

[illuzen]

all duplicates

[gititGoro]

L2 is security related. A new issue has been opened https://github.com/code-423n4/2022-05-factorydao-findings/issues/287