code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-ens/blob/main/contracts/dnssec-oracle/DNSSECImpl.sol#L107 https://github.com/code-423n4/2023-04-ens/blob/main/contracts/dnssec-oracle/DNSSECImpl.sol#L87

Vulnerability details

Impact

The impact of the contract having two different verifyRRSet functions stems from a potential timestamp manipulation vulnerability. The current verifyRRSet function accepts a user-supplied timestamp, which could expose the contract to attacks. If exploited, an attacker could bypass or manipulate the validation process, compromising the function's integrity and potentially causing unintended consequences for the contract and its users.

Proof of Concept

https://github.com/code-423n4/2023-04-ens/blob/main/contracts/dnssec-oracle/DNSSECImpl.sol#L87

 function verifyRRSet(
        RRSetWithSignature[] memory input
    )
        external
        view
        virtual
        override
        returns (bytes memory rrs, uint32 inception)
    {
        return verifyRRSet(input, block.timestamp);
    }

https://github.com/code-423n4/2023-04-ens/blob/main/contracts/dnssec-oracle/DNSSECImpl.sol#L107

  function verifyRRSet(
        RRSetWithSignature[] memory input,
        uint256 now
    )
        public
        view
        virtual
        override
        returns (bytes memory rrs, uint32 inception)
    {
        bytes memory proof = anchors;
        for (uint256 i = 0; i < input.length; i++) {
            RRUtils.SignedSet memory rrset = validateSignedSet(
                input[i],
                proof,
                now
            );
            proof = rrset.data;
            inception = rrset.inception;
        }
        return (proof, inception);
    }

The existing verifyRRSet function can be exploited by providing a manipulated timestamp value. This could potentially enable the attacker to bypass validation checks or modify the function's behavior in an unintended way, leading to a compromised contract state.

Tools Used

Manual review

Recommended Mitigation Steps

It is recommended to use block.timestamp directly as the current timestamp. This approach removes the need for the user to supply a timestamp, mitigating the risk of incorrect or manipulated input.

Implement the following verifyRRSet function, which uses block.timestamp:

function verifyRRSet(RRSetWithSignature[] memory input) public view virtual override returns (bytes memory rrs, uint32 inception) {
    bytes memory proof = anchors;
    uint256 currentTimestamp = block.timestamp;

    for (uint256 i = 0; i < input.length; i++) {
        RRUtils.SignedSet memory rrset = validateSignedSet(input[i], proof, currentTimestamp);
        proof = rrset.data;
        inception = rrset.inception;
    }
    return (proof, inception);
}

this function eliminates the redundancy of having multiple functions with the same name and ensure a secure and accurate timestamp is used within the function.

thereksfour commented 1 year ago

now == block.timestamp, invilid.

    function verifyRRSet(
        RRSetWithSignature[] memory input
    )
        external
        view
        virtual
        override
        returns (bytes memory rrs, uint32 inception)
    {
        return verifyRRSet(input, block.timestamp);
    }

c4-pre-sort commented 1 year ago

thereksfour marked the issue as low quality report

c4-judge commented 1 year ago

dmvt marked the issue as unsatisfactory: Invalid

code-423n4 / 2023-04-ens-findings