Skip to content

function checkUpkeep will revert when inputing a 64 bytes checkdata #3

@CoheeYang

Description

@CoheeYang

Summary

The checkUpkeep across multiple files implemented the same logic to allow users to specify a start index to check if upkeep needed.

    function checkUpkeep(bytes calldata checkData) external view override returns (bool upkeepNeeded, bytes memory performData) {
        uint96 i = 0;
        uint96 length = uint96(dataSet.length());
        if (checkData.length == 64) {
            //decode start and end idxs
            (i, length) = abi.decode(checkData, (uint96, uint96));
            if (length > uint96(dataSet.length())) {
                length = uint96(dataSet.length());
            }
        }
         .....

If the checkData is 64 bytes, then the checkData is decoded into two 12 bytes uint96, which are 24 bytes together, but the abi.decode does not support decode a larger bytes variable into smaller bytes ones, and this function will revert if the automation robot input a checkData with 64 bytes.

Impact

checkUpkeep() is a key function in trading automation to give automation instructions to call performUpkeep after finding the executable orders. However, checkUpkeep() can only find the first executable order in the data set if there is no checkData input.
The if condition is adopted to solve this issue, but the logic is wrong and will never work, which can plague the protocal in efficiency and effectiveness.

PoC

This is very easy to prove, you can copy the following code and run in remix to show the problem.

pragma solidity ^0.8.20;

contract DecodingTest {
    function test64BytesToUint96() public pure returns (uint96, uint96) {
    
        bytes memory data = hex"11111111111111111111111111111111111111111111111111111111111111112222222222222222222222222222222222222222222222222222222222222222";
        require(data.length == 64);
        
        (uint96 first, uint96 second) = abi.decode(data, (uint96, uint96));
        
        return (first, second);
    }
}

Mitigation

change the 64 bytes check to a 24 bytes check.

    function checkUpkeep(bytes calldata checkData) external view override returns (bool upkeepNeeded, bytes memory performData) {
        uint96 i = 0;
        uint96 length = uint96(dataSet.length());
-        if (checkData.length == 64) {
+       if (checkData.length == 24) {
            //decode start and end idxs
            (i, length) = abi.decode(checkData, (uint96, uint96));
            if (length > uint96(dataSet.length())) {
                length = uint96(dataSet.length());
            }
        }
         .....

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions