Lines of code

Vulnerability details

Impact

The BytesUtils.keccak function accepts out of bound offset value and returns a valid response without reverting.

    function keccak(
        bytes memory self,
        uint256 offset,
        uint256 len
    ) internal pure returns (bytes32 ret) {
        require(offset + len <= self.length);
        assembly {
            ret := keccak256(add(add(self, 32), offset), len)
        }
    }

The function contains a require statement which validates offset + len <= self.length condition but it does not revert when offset is provided equal to the string’s length and the len is provided as 0.

This edge case is a bug. The keccak function is used at various places throughout the protocol. The contracts interacting with the keccak function assumes that the function will revert on invalid inputs. Hence the non-reverting flow in the edge case breaks that assumption of other contracts and may lead to serious issues.

Proof of Concept

A mock contract was created to expose the keccak function:

pragma solidity ^0.8.4;
import "./BytesUtils.sol";

contract AuditBytesUtils {
    function keccak(
        bytes memory self,
        uint256 offset,
        uint256 len
    ) public pure returns (bytes32 ret) {
        return BytesUtils.keccak(self, offset, len);
    }
}

A new test file was created and ran using yarn test path-to-file

const { use, expect } = require('chai')
const { solidity } = require('ethereum-waffle')
const { ethers } = require('hardhat')
const { toUtf8Bytes } = require('ethers/lib/utils')

use(solidity)
const utils = ethers.utils

describe('AuditBytesUtils', () => {
    let AuditBytesUtils

    before(async () => {
        const AuditBytesUtilsFactory = await ethers.getContractFactory('AuditBytesUtils')
        AuditBytesUtils = await AuditBytesUtilsFactory.deploy()
    })
    
    it('keccak: Bug - does not revert when start index is out of bounds', async () => {
        const input = toUtf8Bytes('5cee339e13375638553bdf5a6e36ba80fb9f6a4f0783680884d92b558aa471da')
        expect(await AuditBytesUtils.keccak(input, input.length, 0)).to.eq(ethers.utils.id(''))
    })
})

Tools Used

Hardhat

Recommended Mitigation Steps

Consider validating the offset parameter:

    function keccak(
        bytes memory self,
        uint256 offset,
        uint256 len
    ) internal pure returns (bytes32 ret) {
        require(offset < self.length);      // <----- FIX
        require(offset + len <= self.length);
        assembly {
            ret := keccak256(add(add(self, 32), offset), len)
        }
    }  

---

The text was updated successfully, but these errors were encountered:

All reactions