code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

Several instances of loops have been identified within the contract, particularly in the functions verifyWithDS and verifyWithKnownKey. These loops are susceptible to gas cost vulnerabilities if an attacker provides carefully crafted input. By causing the contract to consume a significant amount of gas, an attacker can potentially execute a denial of service (DoS) attack, affecting the contract's availability and functionality for other users.

 function verifyWithDS(
        RRUtils.SignedSet memory rrset,
        RRSetWithSignature memory data,
        RRUtils.RRIterator memory proof
    ) internal view {
        uint256 proofOffset = proof.offset;
        for (
            RRUtils.RRIterator memory iter = rrset.rrs();
            !iter.done();
            iter.next()
        ) {
            if (iter.dnstype != DNSTYPE_DNSKEY) {
                revert InvalidProofType(iter.dnstype);
            }

            bytes memory keyrdata = iter.rdata();
            RRUtils.DNSKEY memory dnskey = keyrdata.readDNSKEY(
                0,
                keyrdata.length
            );
            if (verifySignatureWithKey(dnskey, keyrdata, rrset, data)) {
                // It's self-signed - look for a DS record to verify it.
                if (
                    verifyKeyWithDS(rrset.signerName, proof, dnskey, keyrdata)
                ) {
                    return;
                }
                // Rewind proof iterator to the start for the next loop iteration.
                proof.nextOffset = proofOffset;
                proof.next();
            }
        }
        revert NoMatchingProof(rrset.signerName);
    }

 function verifyWithKnownKey(
        RRUtils.SignedSet memory rrset,
        RRSetWithSignature memory data,
        RRUtils.RRIterator memory proof
    ) internal view {
        // Check the DNSKEY's owner name matches the signer name on the RRSIG
        for (; !proof.done(); proof.next()) {
            bytes memory proofName = proof.name();
            if (!proofName.equals(rrset.signerName)) {
                revert ProofNameMismatch(rrset.signerName, proofName);
            }

            bytes memory keyrdata = proof.rdata();
            RRUtils.DNSKEY memory dnskey = keyrdata.readDNSKEY(
                0,
                keyrdata.length
            );
            if (verifySignatureWithKey(dnskey, keyrdata, rrset, data)) {
                return;
            }
        }
        revert NoMatchingProof(rrset.signerName);
    }

Proof of Concept

An attacker could exploit these vulnerabilities by providing input data designed to maximize the number of loop iterations, thereby increasing gas consumption. For example, consider the following crafted input:

// Exploit code
uint[] memory maliciousInput = new uint[](1000000); // A very large input array
contractInstance.verifyWithDS(maliciousInput); // Trigger the verifyWithDS function with the malicious input array

By using this input, the attacker forces the loop to run for an excessive number of iterations, consuming a large amount of gas and potentially causing a DoS condition for the smart contract.

Tools Used

Manual review

Recommended Mitigation Steps

1- Add limits to the number of iterations a loop can perform:

To mitigate this risk, consider adding limits to the number of iterations a loop can perform within the verifyWithDS and verifyWithKnownKey functions. This can be achieved by introducing a constant representing the maximum allowed iterations and implementing a counter variable to track the current iteration count. For example, you may update the verifyWithDS function as follows:

uint constant MAX_LOOP_ITERATIONS = 100; // Define a reasonable maximum number of loop iterations

function verifyWithDS(uint[] memory inputArray) public {
    uint iterationCount = 0;

    for (uint i = 0; i < inputArray.length; i++) {
        // Rest of the function logic

        iterationCount++;
        if (iterationCount >= MAX_LOOP_ITERATIONS) {
            break; // Exit the loop if the maximum iteration count is reached
        }
    }
}

2 - Use the "break" keyword to exit the loop early if specific conditions are met:

In addition to limiting the number of iterations, you can also use the "break" keyword to exit the loop early when specific conditions are met. This can help reduce gas consumption and prevent a DoS attack. For example, if the verifyWithKnownKey function has a condition that can be used to exit the loop early, you could implement it as follows:

function verifyWithKnownKey(uint[] memory inputArray) public {
    for (uint i = 0; i < inputArray.length; i++) {
        // Rest of the function logic

        if (/* specific exit condition */) {
            break; // Exit the loop early if the exit condition is met
        }
    }
}

By implementing these recommendations, you can effectively mitigate the risk of gas cost vulnerabilities in loops, making it more difficult for an attacker to execute a DoS attack against the contract.