code-423n4 / 2022-09-nouns-builder-findings

10 stars 6 forks source link

Variable data type is not same for FounderParams between IManager.sol and TokenTypesV1.sol #667

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/manager/IManager.sol#L48-L52 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/types/TokenTypesV1.sol#L28-L32

Vulnerability details

Impact

Unnecessary complexity during updating the founder parameters. Refer the below function function _addFounders(IManager.FounderParams[] calldata _founders) internal https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/Token.sol#L71

Proof of Concept

TokenTypesV1.sol#L28-L31

struct Founder { address wallet; uint8 ownershipPct; uint32 vestExpiry; }

IManager.sol#L48-L52

struct FounderParams {
    address wallet;
    uint256 ownershipPct;
    uint256 vestExpiry;
}

Founder : uint8 ownershipPct; uint32 vestExpiry; FounderParams : uint256 ownershipPct; uint256 vestExpiry;

The function _addFounders can be updated/simplified.

Tools Used

Manual code review

Recommended Mitigation Steps

Use the same data type used as in the struct Founder for FounderParams also.

GalloDaSballo commented 2 years ago

Valid finding, Med Severity is beyond exaggerated, downgrading to QA Refactoring

GalloDaSballo commented 2 years ago

R