Incorrect Interface Declaration Causes serialize Function to Always Revert

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x671e1268a8b9fc77943c36cace314563412dfe8b89673d44201da1dff107a478 Severity: medium

Description: Description\ The serialize function in the Script interface is declared with the view state mutability, while the implementations in ScriptP2PKH, ScriptP2SH, and other similar contracts declare the serialize function with the pure state mutability. This inconsistency causes the function to always revert when called through the interface.

Attack Scenario\ Describe how the vulnerability can be exploited.

Attachments

1. Proof of Concept (PoC) File

   incorrect interface declaration

   interface IScript {
   function serialize(bytes memory in_args) external view returns (bytes memory);<-----@audit state mutability should/must be  `pure`
   
   function deserialize(bytes memory script) external pure returns (bytes memory);
   
   function id() external pure returns (bytes4);
   }

   ScriptP2PKH.sol

   function serialize(bytes memory in_args) public pure override returns (bytes memory) {
       (bytes20 pubKeyHash) = abi.decode(in_args, (bytes20));
   
       Buffer.BufferIO memory _buffer = Buffer.alloc(25);
   
       _buffer.write(abi.encodePacked(OP_DUP));
       _buffer.write(abi.encodePacked(OP_HASH160));
       _buffer.write(abi.encodePacked(bytes1(0x14), pubKeyHash));
       _buffer.write(abi.encodePacked(OP_EQUALVERIFY));
       _buffer.write(abi.encodePacked(OP_CHECKSIG));
   
       return _buffer.data;
   }

   ScriptP2SH.sol

   function serialize(bytes memory in_args) public pure override returns (bytes memory) {
       (bytes20 scriptHash) = abi.decode(in_args, (bytes20));
   
       Buffer.BufferIO memory _buffer = Buffer.alloc(23);
   
       _buffer.write(abi.encodePacked(OP_HASH160));
       _buffer.write(abi.encodePacked(bytes1(0x14), scriptHash));
       _buffer.write(abi.encodePacked(OP_EQUAL));
   
       return _buffer.data;
   }

   ScriptP2WPKH.sol

   function serialize(bytes memory in_args) public pure override returns (bytes memory) {
       (bytes20 pubKeyHash) = abi.decode(in_args, (bytes20));
   
       Buffer.BufferIO memory _buffer = Buffer.alloc(22);
   
       _buffer.write(abi.encodePacked(OP_0));
       _buffer.write(abi.encodePacked(bytes1(0x14), pubKeyHash));
   
       return _buffer.data;
   }

   ScriptP2WSH.sol

   function serialize(bytes memory in_args) public pure override returns (bytes memory) {
       (bytes32 scriptHash32) = abi.decode(in_args, (bytes32));
   
       Buffer.BufferIO memory _buffer = Buffer.alloc(34);
   
       _buffer.write(abi.encodePacked(OP_0));
       _buffer.write(abi.encodePacked(bytes1(0x20), scriptHash32));
   
       return _buffer.data;
   }

   when this functions are called in the implementation contracts ,all the functions always reverts

2. Revised Code File (Optional)

   • By updating the serialize function in the Script interface to use the pure state mutability, we ensure that the function signatures match between the interface and the implementing contracts. T

[party-for-illuminati]

Spam

[AresAudits]

@party-for-illuminati sorry my bad. the issue here, is the serialize() function in all the script contracts always reverts due to its state mutability i.e pure. here, the function is reading the variables from the contract link.

 bytes1 public constant OP_HASH160 = 0xa9;
    bytes1 public constant OP_DUP = 0x76;
    bytes1 public constant OP_EQUALVERIFY = 0x88;
    bytes1 public constant OP_CHECKSIG = 0xac;

    function serialize(bytes memory in_args) public pure override returns (bytes memory) {
        (bytes20 pubKeyHash) = abi.decode(in_args, (bytes20));

        Buffer.BufferIO memory _buffer = Buffer.alloc(25);

        _buffer.write(abi.encodePacked(OP_DUP));
        _buffer.write(abi.encodePacked(OP_HASH160));
        _buffer.write(abi.encodePacked(bytes1(0x14), pubKeyHash));
        _buffer.write(abi.encodePacked(OP_EQUALVERIFY));
        _buffer.write(abi.encodePacked(OP_CHECKSIG));

        return _buffer.data;
    }

below is the POC ,which always reverts when the callSerialize function is called due to state mutability

to verify the poc please deploy the below file in theremix-ideand call thecallSerialize()function

// SPDX-License-Identifier: GPL-3.0

pragma solidity >=0.8.2 <0.9.0;

/**
 * @title Storage
 * @dev Store & retrieve value in a variable
 * @custom:dev-run-script ./scripts/deploy_with_ethers.ts
 */
 library Endian {
    function reverse256(uint256 input) internal pure returns (uint256 v) {
        v = input;

        // swap bytes
        uint256 pat1 = 0xFF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00;
        v = ((v & pat1) >> 8) | ((v & ~pat1) << 8);

        // swap 2-byte long pairs
        uint256 pat2 = 0xFFFF0000FFFF0000FFFF0000FFFF0000FFFF0000FFFF0000FFFF0000FFFF0000;
        v = ((v & pat2) >> 16) | ((v & ~pat2) << 16);

        // swap 4-byte long pairs
        uint256 pat4 = 0xFFFFFFFF00000000FFFFFFFF00000000FFFFFFFF00000000FFFFFFFF00000000;
        v = ((v & pat4) >> 32) | ((v & ~pat4) << 32);

        // swap 8-byte long pairs
        uint256 pat8 = 0xFFFFFFFFFFFFFFFF0000000000000000FFFFFFFFFFFFFFFF0000000000000000;
        v = ((v & pat8) >> 64) | ((v & ~pat8) << 64);

        // swap 16-byte long pairs
        v = (v >> 128) | (v << 128);
    }

    function reverse64(uint64 input) internal pure returns (uint64 v) {
        v = input;

        // swap bytes
        v = ((v & 0xFF00FF00FF00FF00) >> 8) | ((v & 0x00FF00FF00FF00FF) << 8);

        // swap 2-byte long pairs
        v = ((v & 0xFFFF0000FFFF0000) >> 16) | ((v & 0x0000FFFF0000FFFF) << 16);

        // swap 4-byte long pairs
        v = (v >> 32) | (v << 32);
    }

    function reverse32(uint32 input) internal pure returns (uint32 v) {
        v = input;

        // swap bytes
        v = ((v & 0xFF00FF00) >> 8) | ((v & 0x00FF00FF) << 8);

        // swap 2-byte long pairs
        v = (v >> 16) | (v << 16);
    }

    function reverse16(uint16 input) internal pure returns (uint16 v) {
        v = input;

        // swap bytes
        v = (v >> 8) | (v << 8);
    }
}

library Buffer {
    struct BufferIO {
        bytes data;
        uint256 cursor;
    }

    function alloc(uint256 size) internal pure returns (BufferIO memory) {
        bytes memory _data = new bytes(size);
        return BufferIO(_data, 0);
    }

    function read(BufferIO memory buffer, uint256 size) internal pure returns (bytes memory) {
        require(buffer.cursor + size <= buffer.data.length, "Invalid buffer size");

        bytes memory slice = new bytes(size);
        for (uint i = buffer.cursor; i < buffer.cursor + size; i++) {
            slice[i - buffer.cursor] = buffer.data[i];
        }

        buffer.cursor += size;
        return slice;
    }

    function write(BufferIO memory buffer, bytes memory data) internal pure {
        require(buffer.cursor + data.length <= buffer.data.length, "Invalid buffer size");

        for (uint i = buffer.cursor; i < buffer.cursor + data.length; i++) {
            buffer.data[i] = data[i - buffer.cursor];
        }

        buffer.cursor += data.length;
    }

    function writeBytes32(BufferIO memory buffer, bytes32 data) internal pure {
        write(buffer, bytes.concat(data));
    }

    function writeUint256(BufferIO memory buffer, uint256 data) internal pure {
        write(buffer, bytes.concat(bytes32(data)));
    }

    function writeUint32(BufferIO memory buffer, uint32 data) internal pure {
        write(buffer, bytes.concat(bytes4(data)));
    }

    function writeUint64(BufferIO memory buffer, uint64 data) internal pure {
        write(buffer, bytes.concat(bytes8(data)));
    }

    function readBytes32(BufferIO memory buffer) internal pure returns (bytes32) {
        return bytes32(read(buffer, 32));
    }

    function readUint256(BufferIO memory buffer) internal pure returns (uint256) {
        return uint256(bytes32(read(buffer, 32)));
    }

    function readUint16(BufferIO memory buffer) internal pure returns (uint16) {
        return uint16(bytes2(read(buffer, 2)));
    }

    function readUint64(BufferIO memory buffer) internal pure returns (uint64) {
        return uint64(bytes8(read(buffer, 8)));
    }

    function readUint32(BufferIO memory buffer) internal pure returns (uint32) {
        return uint32(bytes4(read(buffer, 4)));
    }

    function computeVarIntSize(uint256 value) internal pure returns (uint256) {
        if (value <= 0xFC) {
            return 1;
        } else if (value >= 253 && value <= 0xFFFF) {
            return 2;
        } else if (value >= 65536 && value <= 0xFFFFFFFF) {
            return 4;
        } else if (value >= 4_294_967_296 && value <= 0xFFFFFFFFFFFFFFFF) {
            return 8;
        } else {
            revert("Value too large");
        }
    }

    function writeVarInt(BufferIO memory buffer, uint256 value) internal pure {
        if (value <= 0xFC) {
            write(buffer, bytes.concat(bytes1(uint8(value))));
        } else if (value >= 253 && value <= 0xFFFF) {
            write(buffer, bytes.concat(bytes1(uint8(0xFD))));
            write(buffer, bytes.concat(bytes2(Endian.reverse16(uint16(value)))));
        } else if (value >= 65536 && value <= 0xFFFFFFFF) {
            write(buffer, bytes.concat(bytes1(uint8(0xFE))));
            write(buffer, bytes.concat(bytes4(Endian.reverse32(uint16(value)))));
        } else if (value >= 4_294_967_296 && value <= 0xFFFFFFFFFFFFFFFF) {
            write(buffer, bytes.concat(bytes1(uint8(0xFF))));
            write(buffer, bytes.concat(bytes8(Endian.reverse64(uint64(value)))));
        } else {
            revert("Value too large");
        }
    }

    function readVarInt(BufferIO memory buffer) internal pure returns (uint256) {
        uint8 pivot = uint8(bytes1(read(buffer, 1)));
        if (pivot < 0xFD) {
            return uint256(pivot);
        } else if (pivot == 0xFD) {
            return uint256(Endian.reverse16(readUint16(buffer)));
        } else if (pivot == 0xFE) {
            return uint256(Endian.reverse32(readUint32(buffer)));
        } else if (pivot == 0xFF) {
            return uint256(Endian.reverse64(readUint64(buffer)));
        }

        revert("Invalid VarInt");
    }
}

interface IScript {
    function serialize(bytes memory in_args) external view  returns (bytes memory);

    function deserialize(bytes memory script) external pure returns (bytes memory);

    function id() external pure returns (bytes4);
}

contract ScriptP2PKH is IScript {
    using Buffer for Buffer.BufferIO;

    bytes1 public constant OP_HASH160 = 0xa9;
    bytes1 public constant OP_DUP = 0x76;
    bytes1 public constant OP_EQUALVERIFY = 0x88;
    bytes1 public constant OP_CHECKSIG = 0xac;

    function serialize(bytes memory in_args) public pure override returns (bytes memory) {
        (bytes20 pubKeyHash) = abi.decode(in_args, (bytes20));

        Buffer.BufferIO memory _buffer = Buffer.alloc(25);

        _buffer.write(abi.encodePacked(OP_DUP));
        _buffer.write(abi.encodePacked(OP_HASH160));
        _buffer.write(abi.encodePacked(bytes1(0x14), pubKeyHash));
        _buffer.write(abi.encodePacked(OP_EQUALVERIFY));
        _buffer.write(abi.encodePacked(OP_CHECKSIG));

        return _buffer.data;
    }

    function deserialize(bytes memory script) public pure override returns (bytes memory) {
        bytes memory _pubKeyHash = new bytes(0);

        if (script.length != 25) {
            return _pubKeyHash;
        }

        if (
            script[0] == OP_DUP
            && script[1] == OP_HASH160
            && script[2] == 0x14
            && script[23] == OP_EQUALVERIFY
            && script[24] == OP_CHECKSIG
        ) {
            _pubKeyHash = Buffer.read(Buffer.BufferIO(script, 3), 20);
        }

        return _pubKeyHash;
    }

    function id() public override pure returns (bytes4) {
        return bytes4(keccak256(abi.encodePacked(type(ScriptP2PKH).name)));
    }
}

contract ScriptCaller {
    IScript public script;

    constructor(address _scriptAddress) {
        script = IScript(_scriptAddress);
    }

    function callSerialize() public view returns (bytes memory) {
        bytes memory testInput = "0x12345678";
        return script.serialize(testInput);
    }
}

to verify the poc please deploy the above file in the remix and call the callSerialize() function

[party-for-illuminati]

Your POC is reverting because you are passing incorrect in_args data to serialize()