code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-juicebox/blame/main/juice-buyback/contracts/JBXBuybackDelegate.sol#L359-L362

Vulnerability details

Impact

The current version of the contract uses eip165 to show that it uses IJBFundingCycleDataSource and IJBPayDelegate. However, there is a mistake in the supportsInterface function. As stated in the eip-165 to detect if contract uses eip165 and interfaces need:

Make a STATICCALL to the destination address with input data: 0x01ffc9a701ffc9a700000000000000000000000000000000000000000000000000000000 and gas 30,000. This corresponds to contract.supportsInterface(0x01ffc9a7).
If the call fails or returns false, the destination contract does not implement ERC-165.
If the call returns true, a second call is made with input data 0x01ffc9a7ffffffff00000000000000000000000000000000000000000000000000000000.
If the second call fails or returns true, the destination contract does not implement ERC-165.
Otherwise it implements ERC-165.

In the current implementation call with 0x01ffc9a701ffc9a7 input data will return false which means that the contract does not use eip165.

Proof of Concept


      function supportsInterface(bytes4 _interfaceId) external pure override returns (bool) {
        return _interfaceId == type(IJBFundingCycleDataSource).interfaceId
            || _interfaceId == type(IJBPayDelegate).interfaceId;
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Change current implementation to:


     function supportsInterface(bytes4 _interfaceId) public view virtual override(IERC165) returns (bool) {
        return
            _interfaceId == type(IERC165).interfaceId ||
            _interfaceId == type(IJBFundingCycleDataSource).interfaceId ||
            _interfaceId == type(IJBPayDelegate).interfaceId;

    }

Assessed type

Other

c4-pre-sort commented 1 year ago

dmvt marked the issue as duplicate of #131

c4-judge commented 1 year ago

dmvt changed the severity to QA (Quality Assurance)