ENS Contest - bin2chen's results

Lines of code

https://github.com/code-423n4/2023-04-ens/blob/45ea10bacb2a398e14d711fe28d1738271cd7640/contracts/dnsregistrar/RecordParser.sol#L34

Vulnerability details

Impact

readKeyValue() Possible incorrect read value

Proof of Concept

readKeyValue() is used to read the key and value values and return the offset nextOffset of the next key-value pair

The code is as follows:

    function readKeyValue(
        bytes memory input,
        uint256 offset,
        uint256 len
    )
        internal
        pure
        returns (bytes memory key, bytes memory value, uint256 nextOffset)
    {
        uint256 separator = input.find(offset, len, "=");
        if (separator == type(uint256).max) {
            return ("", "", type(uint256).max);
        }

        uint256 terminator = input.find(
            separator,
            len + offset - separator,
            " "
        );
        if (terminator == type(uint256).max) {
            terminator = input.length;  //<--------@audit Wrong use `input.length`, should use `offset+len`
        }

        key = input.substring(offset, separator - offset);
        value = input.substring(separator + 1, terminator - separator - 1);
        nextOffset = terminator + 1;
    }

As the above code comment, when terminator cannot be found, the wrong setting terminator = input.length, should use terminator= offset + len

Because input is limited to find in the length of len, the current protocol is to reuse bytes to find in a specific range in many places

For example:

input contains three blocks: flag, text, signdata, [] which represents the length of each block, we want to get the second block text inside the key/value

input = [4]flag|[3]a=b|[8]signdata

At this point if we execute readKeyValue(input,6,3), we expect to get key=a and value=b The current implementation will get key=a and value=bsigndata

Tools Used

Recommended Mitigation Steps

    function readKeyValue(
        bytes memory input,
        uint256 offset,
        uint256 len
    )
        internal
        pure
        returns (bytes memory key, bytes memory value, uint256 nextOffset)
    {
        uint256 separator = input.find(offset, len, "=");
        if (separator == type(uint256).max) {
            return ("", "", type(uint256).max);
        }

        uint256 terminator = input.find(
            separator,
            len + offset - separator,
            " "
        );
        if (terminator == type(uint256).max) {
-           terminator = input.length;  
+           terminator= offset + len;
        }

        key = input.substring(offset, separator - offset);
        value = input.substring(separator + 1, terminator - separator - 1);
        nextOffset = terminator + 1;
    }