the `_decodeReceiptUndelegateDone` function is the absence of a check to ensure that the parsing of the CBOR data structure

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x61cae2593e812717fb757bbed8d1619d5902d60e24391fb121301150af423c5c Severity: low

Description: Description\

A significant logical vulnerability in the _decodeReceiptUndelegateDone function is the absence of a check to ensure that the parsing of the CBOR data structure in the result byte array completes properly by processing all bytes in the array. This oversight can allow improperly formatted CBOR data that includes extraneous bytes after a legitimate amount value to be processed without error. Essentially, if the result is padded with additional, unrelated data beyond what is expected, the function will not validate whether it has parsed the entire array, as long as it found an amount key.

Attack Scenario\

1. Crafting Malicious Input: An attacker can exploit this vulnerability by crafting a result byte array that includes a correct CBOR-encoded map with an amount key, followed by extraneous bytes that are irrelevant but won't trigger an error. Particularly:

   • Start with a valid CBOR-encoded map that has the amount key (0xA1) encoding the valid uint128 scalar value.
   • Append additional bytes that are non-conforming with the expected format but don't disrupt reading the initial amount. This could include random bytes or additional CBOR items that are deliberately misleading or malformed.

2. Executing with Crafted Input: When the function _decodeReceiptUndelegateDone is called with this maliciously crafted input, it would successfully decode the amount from the initial part of the data. Given it does not verify that all data in the array was consumed, processing would stop right after reading the amount, disregarding the trailing bytes.

3. Impact:

   • The function completes without any errors since it successfully detects and decodes the amount key.
   • The trailing bytes can be manipulated to perform operations that might remain unnoticed since they aren't validated against any structure or expected format.

Implementation of the Check:

To prevent such exploitation, _decodeReceiptUndelegateDone should incorporate an additional check at the end of the function execution to confirm that offset matches result.length:

if(offset != result.length) revert ExtraneousData();

By adding this check, the function ensures that it has processed exactly all the bytes provided in the input array, thus validating the integrity and format of the CBOR data structure entirely and avoiding exploitation, where extraneous data might be overlooked. This not only ensures stricter adherence to expected input formats but also significantly improves the contract's robustness against malformed or malicious data inputs.

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

[0xRizwan]

An attacker can exploit this vulnerability by crafting a result . . .

_decodeReceiptUndelegateDone() as an internal function is called in consensusTakeReceiptUndelegateDone() function which is further called in emergencyTakeReceiptUndelegateDone() function and it can only be called by contract owner.

    function emergencyTakeReceiptUndelegateDone(uint64 endReceiptId) public onlyOwner returns (uint128 amount) {
        amount = Subcall.consensusTakeReceiptUndelegateDone(endReceiptId);
        emit EmergencyTakeReceiptUndelegateDone(endReceiptId, amount);
    }

result is returned from consensusTakeReceipt() internal function so how an attacker can exploit it when functions are internal and owner protected?? May be i am missing something?