0xShoonya - Hash Collision Vulnerability in `verifyPartyBUpnl` Function of `LibMuon`

[sherlock-admin4]

0xShoonya

High

Hash Collision Vulnerability in verifyPartyBUpnl Function of LibMuon

Vulnerability Details

The verifyPartyBUpnl function in the LibMuon library uses abi.encodePacked to calculate hashes, which can lead to hash collisions due to the presence of dynamic arrays.

    function verifyPartyBUpnl(SingleUpnlSig memory upnlSig, address partyB, address partyA) internal view {
        MuonStorage.Layout storage muonLayout = MuonStorage.layout();
        // == SignatureCheck( ==
        require(block.timestamp <= upnlSig.timestamp + muonLayout.upnlValidTime, "LibMuon: Expired signature");
        // == ) ==
        bytes32 hash = keccak256(
            abi.encodePacked(
                muonLayout.muonAppId,
@>              upnlSig.reqId,
                address(this),
                partyB,
                partyA,
                AccountStorage.layout().partyBNonces[partyB][partyA],
                upnlSig.upnl,
                upnlSig.timestamp,
                getChainId()
            )
        );
        verifyTSSAndGateway(hash, upnlSig.sigs, upnlSig.gatewaySignature);
    }

Specifically, the function’s reliance on this method can result in different structures producing identical hash values, thereby facilitating malicious actions. The use of abi.encodePacked with the dynamic array upnlSig.reqId introduces the risk of hash collisions. Since reqId is a dynamic byte array, two different instances could produce the same serialized byte representation, leading to identical hash values.

Impact

Due to conflicting hash values, signatures can be substituted for each other, making malicious use of illegal signatures possible.

Recommendation

Replace abi.encodePacked with abi.encode to ensure that type information is included in the hash calculation, preventing hash collisions.