Paymaster data is parsed without performing a length check
Description
The parsePaymasterAndData()
function is used to parse the UserOperation
structure's paymasterAndData
field. The paymasterAndData
field can contain data in any format, with the format being defined by the Paymaster itself.
Impact
The function does not perform any length checks on the paymasterAndData
field before attempting to parse it.
function parsePaymasterAndData(
bytes calldata paymasterAndData
)
public pure returns (/* ... */)
{
// [ ... ]
(/* ... */) = abi.decode(
paymasterAndData[VALID_PND_OFFSET:SIGNATURE_OFFSET],
(uint48, uint48, address, address, uint256, uint256)
);
signature = paymasterAndData[SIGNATURE_OFFSET:];
}
In the above case, VALID_PND_OFFSET
is 21
, while SIGNATURE_OFFSET
is 213
. If the paymasterAndData
structure does not contain at least that many bytes in it, then the function will revert.
As this field is fully controllable by a user through the UserOperation
structure, and the parsing is done prior to the signature check in _validatePaymasterUserOp()
, this would allow a user to trigger reverts, which would cause the Entrypoint contract that's calling into _validatePaymasterUserOp()
to waste gas.
Recommendations
Consider adding a check to ensure that the paymasterAndData
structure has the correct length. If it does not, consider returning an error to allow the Entrypoint contract to ignore this UserOperation
and continue.
Remediation
Biconomy Labs implemented a fix for this issue in commit 6787a366↗.