Lack of Zero-Address Validation

Summary

The code analysis of "LibUbiquityPool::setCollateralChainLinkPriceFeed" function reveals a critical vulnerability due to the absence of validation checks for the input parameters "collateralAddress" and "chainLinkPriceFeedAddress". This oversight allows for the possibility of zero addresses being set, which can lead to unintended behaviour or exploitation.

Vulnerability Detail

The function "LibUbiquityPool::setCollateralChainLinkPriceFeed" is designed to set the Chainlink price feed address and staleness threshold for a given collateral. However, it lacks validation checks for the input addresses ("collateralAddress" and "chainLinkPriceFeedAddress"). As a result, it's possible to input a zero address, which could cause the system to behave unpredictably or be exploited for malicious purposes.

Impact

The absence of address validation in the "LibUbiquityPool::setCollateralChainLinkPriceFeed" function can lead to significant security risks, including the potential for system malfunction or vulnerability to attacks. If a zero address is set, it might disrupt the normal operation of the system, leading to financial losses or data integrity issues.

Code Snippet

LoC https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L700

PoC

function testSetCollateralChainLinkPriceFeed_ShouldRevertIfZeroAddress()
        public
    {
        vm.startPrank(admin);

        // vm.expectRevert();
        ubiquityPoolFacet.setCollateralChainLinkPriceFeed(
            address(collateralToken),
            address(0),
            1 days
        );

        vm.stopPrank();
    }

The above test should revert but passes

Tool used

Manual Review

Recommendation

It is recommended to implement validation checks in the "setCollateralChainLinkPriceFeed" function to ensure that neither "collateralAddress" nor "chainLinkPriceFeedAddress" are zero addresses. This can be done using a simple conditional check to revert the transaction if either of the addresses is a zero address. This will significantly enhance the security and robustness of the function.

sherlock-admin2 commented 7 months ago

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

This issue describes about retreiving zero index by default when collateralAddress is unknown, but the function is called by admin so no need to be considered

sherlock-admin2 commented 7 months ago

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

This issue describes about retreiving zero index by default when collateralAddress is unknown, but the function is called by admin so no need to be considered

nevillehuang commented 7 months ago

Invalid, zero address checks are not valid based on sherlock rules

Zero address checks: Check to make sure input values are not zero addresses.

sherlock-audit / 2023-12-ubiquity-judging

0xchromatin - Lack of Zero-Address Validation #137