Decent Sepia Caterpillar
Medium
From the solidity documentation: https://docs.soliditylang.org/en/v0.8.17/abi-spec.html?highlight=collisions#non-standard-packed-mode > If you use keccak256(abi.encodePacked(a, b)) and both a and b are dynamic types, it is easy to craft collisions in the hash value by moving parts of a into b and vice-versa. More specifically, abi.encodePacked("a", "bc") == abi.encodePacked("ab", "c"). This can result in the submission of values that were not actually signed
In function verifySettlement()
function verifySettlement(SettlementSig memory settleSig, address partyA) internal view {
----_SNIP_------
@> encodedData = abi.encodePacked(
encodedData, // Append the previously encoded data
settleSig.quotesSettlementsData[i].quoteId,
settleSig.quotesSettlementsData[i].currentPrice,
settleSig.quotesSettlementsData[i].partyBUpnlIndex
);
}
@> bytes32 hash = keccak256(
abi.encodePacked(
muonLayout.muonAppId,
settleSig.reqId,
address(this),
"verifySettlement",
nonces,
AccountStorage.layout().partyANonces[partyA],
@> encodedData,
settleSig.upnlPartyBs,
settleSig.upnlPartyA,
settleSig.timestamp,
LibMuon.getChainId()
)
);
LibMuon.verifyTSSAndGateway(hash, settleSig.sigs, settleSig.gatewaySignature);
}
}As the solidity docs describe, two or more dynamic types are passed to abi.encodePacked. Moreover, the dynamic value settleSig is user-specified function arguments in the function above , meaning anyone can directly specify the value of these arguments when calling the function
No response
No response
No response
This can result in the submission of values that were not actually signed
No response
Instead of writing functions to accept several arguments that are hashed inside the function, consider rewriting the function to take the hashed value as a function argument directly so that the hashing process happens off-chain. This approach would solve the issue and save gas.