Unhandled failure in OffchainDNSResolver for invalid addresses in DNS records

[tinchoabbate]

I was just checking the newish OffchainDNSResolver contract, and there seems to be an unhandled failure in the OffchainDNSResolver when a user includes an invalid resolver address as part of their DNS record.

As far as I understand, the root problem is in the way OffchainDNSResolver::parseAndResolve attempts to parse the user-provided value to an address.
Whatever value is passed in the nameOrAddress parameter, as long as it starts with 0x, it will be passed down to the HexUtils::hexToAddress function. Where it will attempt to cast it to an Ethereum address. Internally, this function never checks whether the length of the supplied data is longer than an address. It just casts it to a bytes32 value, and then to address.

As an example, the string 0x00FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF00 would be considered to be the address 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF00. This means that OffchainDNSResolver::parseRR would return a non-zero address (which is interpreted as a signal for a valid record found) and the name would try to be resolved calling the 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF00 account.

If the called account happens to be empty (most likely the case if the record was just set incorrectly due to a user's oversight), then execution is reverted with error "function returned an unexpected amount of data" instead of the more gracious CouldNotResolve(bytes) error. This is because the call to IERC165(dnsresolver).supportsInterface(...) is using the explicit cast to the IERC165 interface, which is always expecting a boolean in return - reverting otherwise.

Perhaps the code could be changed to fail earlier. Non-addresses shouldn't be silently parsed as addresses.

Once that's fixed, we'd be left with the scenario where the user sets a valid address in the record, but that address corresponds to a custom resolver that does not follow IERC165. If you want to support that case (which appears to be so, given this fallback-like staticcall), then instead of explicitly casting to IERC165 you might want to consider using the ERC165Checker lib or anything similar that does not revert, but instead allows you to explicitly handle errors when querying unsupported interfaces.